* [PATCH v4 07/14] NFC: nxp-nci: Get rid of useless label
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
Sedat Dilek
Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>
Return directly in ->probe() since there no special cleaning is needed.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
drivers/nfc/nxp-nci/i2c.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 6a627d1b6f85..bec9b1ea78e2 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -265,16 +265,13 @@ static int nxp_nci_i2c_probe(struct i2c_client *client,
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
nfc_err(&client->dev, "Need I2C_FUNC_I2C\n");
- r = -ENODEV;
- goto probe_exit;
+ return -ENODEV;
}
phy = devm_kzalloc(&client->dev, sizeof(struct nxp_nci_i2c_phy),
GFP_KERNEL);
- if (!phy) {
- r = -ENOMEM;
- goto probe_exit;
- }
+ if (!phy)
+ return -ENOMEM;
phy->i2c_dev = client;
i2c_set_clientdata(client, phy);
@@ -298,7 +295,7 @@ static int nxp_nci_i2c_probe(struct i2c_client *client,
r = nxp_nci_probe(phy, &client->dev, &i2c_phy_ops,
NXP_NCI_I2C_MAX_PAYLOAD, &phy->ndev);
if (r < 0)
- goto probe_exit;
+ return r;
r = request_threaded_irq(client->irq, NULL,
nxp_nci_i2c_irq_thread_fn,
@@ -307,7 +304,6 @@ static int nxp_nci_i2c_probe(struct i2c_client *client,
if (r < 0)
nfc_err(&client->dev, "Unable to register IRQ handler\n");
-probe_exit:
return r;
}
--
2.20.1
^ permalink raw reply related
* [PATCH v4 05/14] NFC: nxp-nci: Add GPIO ACPI mapping table
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
Sedat Dilek
Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>
In order to unify GPIO resource request prepare gpiod_get_index()
to behave correctly when there is no mapping provided by firmware.
Here we add explicit mapping between _CRS GpioIo() resources and
their names used in the driver.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
drivers/nfc/nxp-nci/i2c.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 713c267acf88..7344405feddf 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -247,6 +247,15 @@ static irqreturn_t nxp_nci_i2c_irq_thread_fn(int irq, void *phy_id)
return IRQ_NONE;
}
+static const struct acpi_gpio_params firmware_gpios = { 1, 0, false };
+static const struct acpi_gpio_params enable_gpios = { 2, 0, false };
+
+static const struct acpi_gpio_mapping acpi_nxp_nci_gpios[] = {
+ { "enable-gpios", &enable_gpios, 1 },
+ { "firmware-gpios", &firmware_gpios, 1 },
+ { }
+};
+
static int nxp_nci_i2c_parse_devtree(struct i2c_client *client)
{
struct nxp_nci_i2c_phy *phy = i2c_get_clientdata(client);
@@ -269,9 +278,14 @@ static int nxp_nci_i2c_parse_devtree(struct i2c_client *client)
static int nxp_nci_i2c_acpi_config(struct nxp_nci_i2c_phy *phy)
{
struct i2c_client *client = phy->i2c_dev;
+ int r;
- phy->gpiod_en = devm_gpiod_get_index(&client->dev, NULL, 2, GPIOD_OUT_LOW);
- phy->gpiod_fw = devm_gpiod_get_index(&client->dev, NULL, 1, GPIOD_OUT_LOW);
+ r = devm_acpi_dev_add_driver_gpios(&client->dev, acpi_nxp_nci_gpios);
+ if (r)
+ return r;
+
+ phy->gpiod_en = devm_gpiod_get(&client->dev, "enable", GPIOD_OUT_LOW);
+ phy->gpiod_fw = devm_gpiod_get(&client->dev, "firmware", GPIOD_OUT_LOW);
if (IS_ERR(phy->gpiod_en) || IS_ERR(phy->gpiod_fw)) {
nfc_err(&client->dev, "No GPIOs\n");
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
From: Allan W. Nielsen @ 2019-07-29 14:35 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Horatiu Vultur, roopa, davem, bridge, netdev, linux-kernel
In-Reply-To: <e4cd0db9-695a-82a7-7dc0-623ded66a4e5@cumulusnetworks.com>
The 07/29/2019 17:21, Nikolay Aleksandrov wrote:
> On 29/07/2019 16:52, Allan W. Nielsen wrote:
> > The 07/29/2019 15:50, Nikolay Aleksandrov wrote:
> >> On 29/07/2019 15:22, Nikolay Aleksandrov wrote:
> >>> Hi Allan,
> >>> On 29/07/2019 15:14, Allan W. Nielsen wrote:
> >>>> First of all, as mentioned further down in this thread, I realized that our
> >>>> implementation of the multicast floodmasks does not align with the existing SW
> >>>> implementation. We will change this, such that all multicast packets goes to the
> >>>> SW bridge.
> >>>>
> >>>> This changes things a bit, not that much.
> >>>>
> >>>> I actually think you summarized the issue we have (after changing to multicast
> >>>> flood-masks) right here:
> >>>>
> >>>> The 07/26/2019 12:26, Nikolay Aleksandrov wrote:
> >>>>>>> Actually you mentioned non-IP traffic, so the querier stuff is not a problem. This
> >>>>>>> traffic will always be flooded by the bridge (and also a copy will be locally sent up).
> >>>>>>> Thus only the flooding may need to be controlled.
> >>>>
> >>>> This seems to be exactly what we need.
> >>>>
> >>>> Assuming we have a SW bridge (br0) with 4 slave interfaces (eth0-3). We use this
> >>>> on a network where we want to limit the flooding of frames with dmac
> >>>> 01:21:6C:00:00:01 (which is non IP traffic) to eth0 and eth1.
> >>>>
> >>>> One way of doing this could potentially be to support the following command:
> >>>>
> >>>> bridge fdb add 01:21:6C:00:00:01 port eth0
> >>>> bridge fdb append 01:21:6C:00:00:01 port eth1
> >> And the fdbs become linked lists?
> > Yes, it will most likely become a linked list
> >
> >> So we'll increase the complexity for something that is already supported by
> >> ACLs (e.g. tc) and also bridge per-port multicast flood flag ?
> > I do not think it can be supported with the facilities we have today in tc.
> >
> > We can do half of it (copy more fraems to the CPU) with tc, but we can not limit
> > the floodmask of a frame with tc (say we want it to flood to 2 out of 4 slave
> > ports).
> Why not ? You attach an egress filter for the ports and allow that dmac on only
> 2 of the ports.
Because we want a solution which we eventually can offload in HW. And the HW
facilities we have is doing ingress processing (we have no egress ACLs in this
design), and if we try to offload an egress rule, with an ingress HW facility,
then we will run into other issues.
/Allan
^ permalink raw reply
* [PATCH v4 14/14] NFC: nxp-nci: Fix recommendation for NFC_NXP_NCI_I2C Kconfig
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
Sedat Dilek
Cc: Andy Shevchenko, Oleg Zhurakivskyy
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>
From: Sedat Dilek <sedat.dilek@credativ.de>
This is a simple cleanup to the Kconfig help text as discussed in [1].
[1] https://marc.info/?t=155774435600001&r=1&w=2
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Suggested-by: Oleg Zhurakivskyy <oleg.zhurakivskyy@intel.com>
Signed-off-by: Sedat Dilek <sedat.dilek@credativ.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/nfc/nxp-nci/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nfc/nxp-nci/Kconfig b/drivers/nfc/nxp-nci/Kconfig
index 746b91aa74f0..e1f71deab6fc 100644
--- a/drivers/nfc/nxp-nci/Kconfig
+++ b/drivers/nfc/nxp-nci/Kconfig
@@ -22,4 +22,4 @@ config NFC_NXP_NCI_I2C
To compile this driver as a module, choose m here. The module will
be called nxp_nci_i2c.
- Say Y if unsure.
+ Say N if unsure.
--
2.20.1
^ permalink raw reply related
* [PATCH v4 08/14] NFC: nxp-nci: Constify acpi_device_id
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
Sedat Dilek
Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>
The content of acpi_device_id is not supposed to change at runtime.
All functions working with acpi_device_id provided by <linux/acpi.h>
work with const acpi_device_id. So mark the non-const structs as const.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
drivers/nfc/nxp-nci/i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index bec9b1ea78e2..4e71962dc557 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -330,7 +330,7 @@ static const struct of_device_id of_nxp_nci_i2c_match[] = {
MODULE_DEVICE_TABLE(of, of_nxp_nci_i2c_match);
#ifdef CONFIG_ACPI
-static struct acpi_device_id acpi_id[] = {
+static const struct acpi_device_id acpi_id[] = {
{ "NXP1001" },
{ "NXP7471" },
{ },
--
2.20.1
^ permalink raw reply related
* [PATCH v4 11/14] NFC: nxp-nci: Remove unused macro pr_fmt()
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
Sedat Dilek
Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>
The macro had never been used.
The driver uses mostly the nfc_err(), which, with other macros in the family,
is backed by corresponding dev_err(). pr_fmt() is not used for dev_err()
macro. Moreover, there is no need to print the module name which is part of the
device instance name anyway.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
drivers/nfc/nxp-nci/i2c.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 59b0a02a813d..307bd2afbe05 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -12,8 +12,6 @@
* Copyright (C) 2012 Intel Corporation. All rights reserved.
*/
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
#include <linux/acpi.h>
#include <linux/delay.h>
#include <linux/i2c.h>
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
From: Nikolay Aleksandrov @ 2019-07-29 14:21 UTC (permalink / raw)
To: Allan W. Nielsen
Cc: Horatiu Vultur, roopa, davem, bridge, netdev, linux-kernel
In-Reply-To: <20190729135205.oiuthcyesal4b4ct@lx-anielsen.microsemi.net>
On 29/07/2019 16:52, Allan W. Nielsen wrote:
> The 07/29/2019 15:50, Nikolay Aleksandrov wrote:
>> On 29/07/2019 15:22, Nikolay Aleksandrov wrote:
>>> Hi Allan,
>>> On 29/07/2019 15:14, Allan W. Nielsen wrote:
>>>> First of all, as mentioned further down in this thread, I realized that our
>>>> implementation of the multicast floodmasks does not align with the existing SW
>>>> implementation. We will change this, such that all multicast packets goes to the
>>>> SW bridge.
>>>>
>>>> This changes things a bit, not that much.
>>>>
>>>> I actually think you summarized the issue we have (after changing to multicast
>>>> flood-masks) right here:
>>>>
>>>> The 07/26/2019 12:26, Nikolay Aleksandrov wrote:
>>>>>>> Actually you mentioned non-IP traffic, so the querier stuff is not a problem. This
>>>>>>> traffic will always be flooded by the bridge (and also a copy will be locally sent up).
>>>>>>> Thus only the flooding may need to be controlled.
>>>>
>>>> This seems to be exactly what we need.
>>>>
>>>> Assuming we have a SW bridge (br0) with 4 slave interfaces (eth0-3). We use this
>>>> on a network where we want to limit the flooding of frames with dmac
>>>> 01:21:6C:00:00:01 (which is non IP traffic) to eth0 and eth1.
>>>>
>>>> One way of doing this could potentially be to support the following command:
>>>>
>>>> bridge fdb add 01:21:6C:00:00:01 port eth0
>>>> bridge fdb append 01:21:6C:00:00:01 port eth1
>>>>
>>
>> And the fdbs become linked lists?
> Yes, it will most likely become a linked list
>
>> So we'll increase the complexity for something that is already supported by
>> ACLs (e.g. tc) and also bridge per-port multicast flood flag ?
> I do not think it can be supported with the facilities we have today in tc.
>
> We can do half of it (copy more fraems to the CPU) with tc, but we can not limit
> the floodmask of a frame with tc (say we want it to flood to 2 out of 4 slave
> ports).
>
Why not ? You attach an egress filter for the ports and allow that dmac on only
2 of the ports.
>> I'm sorry but that doesn't sound good to me for a case which is very rare and
>> there are existing ways to solve without incurring performance hits or increasing
>> code complexity.
> I do not consider it rarely, controling the forwarding of L2 multicast frames is
> quite common in the applications we are doing.
>
>> If you find a way to achieve this without incurring a performance hit or significant
>> code complexity increase, and without breaking current use-cases (e.g. unexpected default
>> forwarding behaviour changes) then please send a patch and we can discuss it further with
>> all details present. People have provided enough alternatives which avoid all of the
>> problems.
> Will do, thanks for the guidance.
>
> /Allan
>
^ permalink raw reply
* Re: [PATCH v6 rdma-next 1/6] RDMA/core: Create mmap database and cookie helper functions
From: Jason Gunthorpe @ 2019-07-29 14:11 UTC (permalink / raw)
To: Kamal Heib
Cc: Michal Kalderon, ariel.elior, dledford, galpress, linux-rdma,
davem, netdev
In-Reply-To: <20190728093051.GB5250@kheib-workstation>
On Sun, Jul 28, 2019 at 12:30:51PM +0300, Kamal Heib wrote:
> > Maybe put this in ib_core_uverbs.c ?
> >
> > Kamal, you've been tackling various cleanups, maybe making ib_uverbs
> > unloadable again is something you'd be keen on?
> >
>
> Yes, Could you please give some background on that?
Most of it is my fault from being too careless, but the general notion
is that all of these
$ grep EXPORT_SYMBOL uverbs_main.c uverbs_cmd.c uverbs_marshall.c rdma_core.c uverbs_std_types*.c uverbs_uapi.c
uverbs_main.c:EXPORT_SYMBOL(ib_uverbs_get_ucontext_file);
uverbs_main.c:EXPORT_SYMBOL(rdma_user_mmap_io);
uverbs_cmd.c:EXPORT_SYMBOL(flow_resources_alloc);
uverbs_cmd.c:EXPORT_SYMBOL(ib_uverbs_flow_resources_free);
uverbs_cmd.c:EXPORT_SYMBOL(flow_resources_add);
uverbs_marshall.c:EXPORT_SYMBOL(ib_copy_ah_attr_to_user);
uverbs_marshall.c:EXPORT_SYMBOL(ib_copy_qp_attr_to_user);
uverbs_marshall.c:EXPORT_SYMBOL(ib_copy_path_rec_to_user);
uverbs_marshall.c:EXPORT_SYMBOL(ib_copy_path_rec_from_user);
rdma_core.c:EXPORT_SYMBOL(uverbs_idr_class);
rdma_core.c:EXPORT_SYMBOL(uverbs_close_fd);
rdma_core.c:EXPORT_SYMBOL(uverbs_fd_class);
uverbs_std_types.c:EXPORT_SYMBOL(uverbs_destroy_def_handler);
Need to go into some 'ib_core uverbs support' .c file in the ib_core,
be moved to a header inline, or moved otherwise
Maybe it is now unrealistic that the uapi is so complicated, ie
uverbs_close_fd is just not easy to fixup..
Maybe the only ones that need fixing are ib_uverbs_get_ucontext_file
rdma_user_mmap_io as alot of drivers are entangled on those now.
The other stuff is much harder..
Jason
^ permalink raw reply
* RE: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jose Abreu @ 2019-07-29 14:08 UTC (permalink / raw)
To: Robin Murphy, Jose Abreu, Jon Hunter,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Catalin Marinas,
Will Deacon
Cc: Joao Pinto, Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai,
Maxime Coquelin, linux-tegra, Giuseppe Cavallaro,
David S . Miller
In-Reply-To: <fcf648d2-70cc-d734-871a-ca7f745791b7@arm.com>
[-- Attachment #1: Type: text/plain, Size: 4612 bytes --]
From: Robin Murphy <robin.murphy@arm.com>
Date: Jul/29/2019, 12:52:02 (UTC+00:00)
> On 29/07/2019 12:29, Jose Abreu wrote:
> > ++ Catalin, Will (ARM64 Maintainers)
> >
> > From: Jon Hunter <jonathanh@nvidia.com>
> > Date: Jul/29/2019, 11:55:18 (UTC+00:00)
> >
> >>
> >> On 29/07/2019 09:16, Jose Abreu wrote:
> >>> From: Jose Abreu <joabreu@synopsys.com>
> >>> Date: Jul/27/2019, 16:56:37 (UTC+00:00)
> >>>
> >>>> From: Jon Hunter <jonathanh@nvidia.com>
> >>>> Date: Jul/26/2019, 15:11:00 (UTC+00:00)
> >>>>
> >>>>>
> >>>>> On 25/07/2019 16:12, Jose Abreu wrote:
> >>>>>> From: Jon Hunter <jonathanh@nvidia.com>
> >>>>>> Date: Jul/25/2019, 15:25:59 (UTC+00:00)
> >>>>>>
> >>>>>>>
> >>>>>>> On 25/07/2019 14:26, Jose Abreu wrote:
> >>>>>>>
> >>>>>>> ...
> >>>>>>>
> >>>>>>>> Well, I wasn't expecting that :/
> >>>>>>>>
> >>>>>>>> Per documentation of barriers I think we should set descriptor fields
> >>>>>>>> and then barrier and finally ownership to HW so that remaining fields
> >>>>>>>> are coherent before owner is set.
> >>>>>>>>
> >>>>>>>> Anyway, can you also add a dma_rmb() after the call to
> >>>>>>>> stmmac_rx_status() ?
> >>>>>>>
> >>>>>>> Yes. I removed the debug print added the barrier, but that did not help.
> >>>>>>
> >>>>>> So, I was finally able to setup NFS using your replicated setup and I
> >>>>>> can't see the issue :(
> >>>>>>
> >>>>>> The only difference I have from yours is that I'm using TCP in NFS
> >>>>>> whilst you (I believe from the logs), use UDP.
> >>>>>
> >>>>> So I tried TCP by setting the kernel boot params to 'nfsvers=3' and
> >>>>> 'proto=tcp' and this does appear to be more stable, but not 100% stable.
> >>>>> It still appears to fail in the same place about 50% of the time.
> >>>>>
> >>>>>> You do have flow control active right ? And your HW FIFO size is >= 4k ?
> >>>>>
> >>>>> How can I verify if flow control is active?
> >>>>
> >>>> You can check it by dumping register MTL_RxQ_Operation_Mode (0xd30).
> >>
> >> Where would be the appropriate place to dump this? After probe? Maybe
> >> best if you can share a code snippet of where to dump this.
> >>
> >>>> Can you also add IOMMU debug in file "drivers/iommu/iommu.c" ?
> >>
> >> You can find a boot log here:
> >>
> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__paste.ubuntu.com_p_qtRqtYKHGF_&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=NrxsR2etpZHGb7HkN4XdgaGmKM1XYyldihNPL6qVSv0&s=CMATEcHVoqZw4sIrNOXc7SFE_kV_5CO5EU21-yJez6c&e=
> >>
> >>> And, please try attached debug patch.
> >>
> >> With this patch it appears to boot fine. So far no issues seen.
> >
> > Thank you for testing.
> >
> > Hi Catalin and Will,
> >
> > Sorry to add you in such a long thread but we are seeing a DMA issue
> > with stmmac driver in an ARM64 platform with IOMMU enabled.
> >
> > The issue seems to be solved when buffers allocation for DMA based
> > transfers are *not* mapped with the DMA_ATTR_SKIP_CPU_SYNC flag *OR*
> > when IOMMU is disabled.
> >
> > Notice that after transfer is done we do use
> > dma_sync_single_for_{cpu,device} and then we reuse *the same* page for
> > another transfer.
> >
> > Can you please comment on whether DMA_ATTR_SKIP_CPU_SYNC can not be used
> > in ARM64 platforms with IOMMU ?
>
> In terms of what they do, there should be no difference on arm64 between:
>
> dma_map_page(..., dir);
> ...
> dma_unmap_page(..., dir);
>
> and:
>
> dma_map_page_attrs(..., dir, DMA_ATTR_SKIP_CPU_SYNC);
> dma_sync_single_for_device(..., dir);
> ...
> dma_sync_single_for_cpu(..., dir);
> dma_unmap_page_attrs(..., dir, DMA_ATTR_SKIP_CPU_SYNC);
>
> provided that the first sync covers the whole buffer and any subsequent
> ones cover at least the parts of the buffer which may have changed. Plus
> for coherent hardware it's entirely moot either way.
Thanks for confirming. That's indeed what stmmac is doing when buffer is
received by syncing the packet size to CPU.
>
> Given Jon's previous findings, I would lean towards the idea that
> performing the extra (redundant) cache maintenance plus barrier in
> dma_unmap is mostly just perturbing timing in the same way as the debug
> print which also made things seem OK.
Mikko said that Tegra186 is not coherent so we have to explicit flush
pipeline but I don't understand why sync_single() is not doing it ...
Jon, can you please remove *all* debug prints, hacks, etc ... and test
this one in attach with plain -net tree ?
---
Thanks,
Jose Miguel Abreu
[-- Attachment #2: 0001-net-stmmac-Flush-all-data-cache-in-RX-path.patch --]
[-- Type: application/octet-stream, Size: 1647 bytes --]
From 1b512c799cd896c7b609be512db7c477def43c6b Mon Sep 17 00:00:00 2001
Message-Id: <1b512c799cd896c7b609be512db7c477def43c6b.1564408914.git.joabreu@synopsys.com>
From: Jose Abreu <joabreu@synopsys.com>
Date: Mon, 29 Jul 2019 16:01:36 +0200
Subject: [PATCH net] net: stmmac: Flush all data cache in RX path
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
---
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 98b1a5c6d537..ed7f0d6bd0bc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -27,6 +27,7 @@
#include <linux/if.h>
#include <linux/if_vlan.h>
#include <linux/dma-mapping.h>
+#include <linux/dma-noncoherent.h>
#include <linux/slab.h>
#include <linux/prefetch.h>
#include <linux/pinctrl/consumer.h>
@@ -3420,6 +3421,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
continue;
}
+ arch_dma_prep_coherent(buf->page, frame_len);
+
dma_sync_single_for_cpu(priv->device, buf->addr,
frame_len, DMA_FROM_DEVICE);
skb_copy_to_linear_data(skb, page_address(buf->page),
--
2.7.4
^ permalink raw reply related
* RE: [PATCH v6 rdma-next 1/6] RDMA/core: Create mmap database and cookie helper functions
From: Michal Kalderon @ 2019-07-29 14:07 UTC (permalink / raw)
To: Gal Pressman, Jason Gunthorpe
Cc: Ariel Elior, dledford@redhat.com, linux-rdma@vger.kernel.org,
davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <d632598e-0896-fa10-9148-73794a9a49d7@amazon.com>
> From: Gal Pressman <galpress@amazon.com>
> Sent: Monday, July 29, 2019 4:54 PM
>
> On 29/07/2019 15:58, Michal Kalderon wrote:
> >> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> >> owner@vger.kernel.org> On Behalf Of Jason Gunthorpe
> >>
> >>> + xa_lock(&ucontext->mmap_xa);
> >>> + if (check_add_overflow(ucontext->mmap_xa_page,
> >>> + (u32)(length >> PAGE_SHIFT),
> >>> + &next_mmap_page))
> >>> + goto err_unlock;
> >>
> >> I still don't like that this algorithm latches into a permanent
> >> failure when the xa_page wraps.
> >>
> >> It seems worth spending a bit more time here to tidy this.. Keep
> >> using the mmap_xa_page scheme, but instead do something like
> >>
> >> alloc_cyclic_range():
> >>
> >> while () {
> >> // Find first empty element in a cyclic way
> >> xa_page_first = mmap_xa_page;
> >> xa_find(xa, &xa_page_first, U32_MAX, XA_FREE_MARK)
> >>
> >> // Is there a enough room to have the range?
> >> if (check_add_overflow(xa_page_first, npages, &xa_page_end)) {
> >> mmap_xa_page = 0;
> >> continue;
> >> }
> >>
> >> // See if the element before intersects
> >> elm = xa_find(xa, &zero, xa_page_end, 0);
> >> if (elm && intersects(xa_page_first, xa_page_last, elm->first, elm-
> >last)) {
> >> mmap_xa_page = elm->last + 1;
> >> continue
> >> }
> >>
> >> // xa_page_first -> xa_page_end should now be free
> >> xa_insert(xa, xa_page_start, entry);
> >> mmap_xa_page = xa_page_end + 1;
> >> return xa_page_start;
> >> }
> >>
> >> Approximately, please check it.
> > Gal & Jason,
> >
> > Coming back to the mmap_xa_page algorithm. I couldn't find some
> background on this.
> > Why do you need the length to be represented in the mmap_xa_page ?
> > Why not simply use xa_alloc_cyclic ( like in siw ) This is simply a
> > key to a mmap object...
>
> The intention was that the entry would "occupy" number of xarray elements
> according to its size (in pages). It wasn't initially like this, but IIRC this was
> preferred by Jason.
Thanks, so Jason, if we're now freeing the objects, can we simply us xa_alloc_cyclic instead?
Thanks,
Michal
^ permalink raw reply
* Re: [PATCH v6 rdma-next 1/6] RDMA/core: Create mmap database and cookie helper functions
From: Jason Gunthorpe @ 2019-07-29 14:06 UTC (permalink / raw)
To: Gal Pressman
Cc: Michal Kalderon, Kamal Heib, Ariel Elior, dledford@redhat.com,
linux-rdma@vger.kernel.org, davem@davemloft.net,
netdev@vger.kernel.org
In-Reply-To: <1e54c4de-7349-3154-1b98-39774c83899f@amazon.com>
On Sun, Jul 28, 2019 at 11:45:56AM +0300, Gal Pressman wrote:
> On 26/07/2019 16:23, Jason Gunthorpe wrote:
> > On Fri, Jul 26, 2019 at 08:42:07AM +0000, Michal Kalderon wrote:
> >
> >>>> But we don't free entires from the xa_array ( only when ucontext is
> >>>> destroyed) so how will There be an empty element after we wrap ?
> >>>
> >>> Oh!
> >>>
> >>> That should be fixed up too, in the general case if a user is
> >>> creating/destroying driver objects in loop we don't want memory usage to
> >>> be unbounded.
> >>>
> >>> The rdma_user_mmap stuff has VMA ops that can refcount the xa entry and
> >>> now that this is core code it is easy enough to harmonize the two things and
> >>> track the xa side from the struct rdma_umap_priv
> >>>
> >>> The question is, does EFA or qedr have a use model for this that allows a
> >>> userspace verb to create/destroy in a loop? ie do we need to fix this right
> >>> now?
> >
> >> The mapping occurs for every qp and cq creation. So yes.
> >>
> >> So do you mean add a ref-cnt to the xarray entry and from umap
> >> decrease the refcnt and free?
> >
> > Yes, free the entry (release the HW resource) and release the xa_array
> > ID.
>
> This is a bit tricky for EFA.
> The UAR BAR resources (LLQ for example) aren't cleaned up until the UAR is
> deallocated, so many of the entries won't really be freed when the refcount
> reaches zero (i.e the HW considers these entries as refcounted as long as the
> UAR exists). The best we can do is free the DMA buffers for appropriate entries.
Drivers can still defer HW destruction until the ucontext destroys,
but this gives an option to move it sooner, which looks like the other
drivers do need as they can allocate these things in userspace loops.
Jason
^ permalink raw reply
* Re: [PATCH v6 rdma-next 1/6] RDMA/core: Create mmap database and cookie helper functions
From: Jason Gunthorpe @ 2019-07-29 14:04 UTC (permalink / raw)
To: Gal Pressman
Cc: Michal Kalderon, Ariel Elior, dledford@redhat.com,
linux-rdma@vger.kernel.org, davem@davemloft.net,
netdev@vger.kernel.org
In-Reply-To: <d632598e-0896-fa10-9148-73794a9a49d7@amazon.com>
On Mon, Jul 29, 2019 at 04:53:38PM +0300, Gal Pressman wrote:
> On 29/07/2019 15:58, Michal Kalderon wrote:
> >> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> >> owner@vger.kernel.org> On Behalf Of Jason Gunthorpe
> >>
> >>> + xa_lock(&ucontext->mmap_xa);
> >>> + if (check_add_overflow(ucontext->mmap_xa_page,
> >>> + (u32)(length >> PAGE_SHIFT),
> >>> + &next_mmap_page))
> >>> + goto err_unlock;
> >>
> >> I still don't like that this algorithm latches into a permanent failure when the
> >> xa_page wraps.
> >>
> >> It seems worth spending a bit more time here to tidy this.. Keep using the
> >> mmap_xa_page scheme, but instead do something like
> >>
> >> alloc_cyclic_range():
> >>
> >> while () {
> >> // Find first empty element in a cyclic way
> >> xa_page_first = mmap_xa_page;
> >> xa_find(xa, &xa_page_first, U32_MAX, XA_FREE_MARK)
> >>
> >> // Is there a enough room to have the range?
> >> if (check_add_overflow(xa_page_first, npages, &xa_page_end)) {
> >> mmap_xa_page = 0;
> >> continue;
> >> }
> >>
> >> // See if the element before intersects
> >> elm = xa_find(xa, &zero, xa_page_end, 0);
> >> if (elm && intersects(xa_page_first, xa_page_last, elm->first, elm->last)) {
> >> mmap_xa_page = elm->last + 1;
> >> continue
> >> }
> >>
> >> // xa_page_first -> xa_page_end should now be free
> >> xa_insert(xa, xa_page_start, entry);
> >> mmap_xa_page = xa_page_end + 1;
> >> return xa_page_start;
> >> }
> >>
> >> Approximately, please check it.
> > Gal & Jason,
> >
> > Coming back to the mmap_xa_page algorithm. I couldn't find some background on this.
> > Why do you need the length to be represented in the mmap_xa_page ?
> > Why not simply use xa_alloc_cyclic ( like in siw )
I think siw is dealing with only PAGE_SIZE objects, efa had variable
sized ones.
> > This is simply a key to a mmap object...
>
> The intention was that the entry would "occupy" number of xarray elements
> according to its size (in pages). It wasn't initially like this, but IIRC this
> was preferred by Jason.
It is not so critical, maybe we could drop it if it is really
simplifiying. But it doesn't look so hard to make an xa algorithm that
will be OK.
The offset/length is shown in things like lsof and what not, and from
a debugging perspective it makes a lot more sense if the offset/length
are sensible, ie they should not overlap.
Jason
^ permalink raw reply
* Re: [PATCH v4 1/5] vsock/virtio: limit the memory used per-socket
From: Michael S. Tsirkin @ 2019-07-29 14:04 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, linux-kernel, Stefan Hajnoczi, David S. Miller,
virtualization, Jason Wang, kvm
In-Reply-To: <20190717113030.163499-2-sgarzare@redhat.com>
On Wed, Jul 17, 2019 at 01:30:26PM +0200, Stefano Garzarella wrote:
> Since virtio-vsock was introduced, the buffers filled by the host
> and pushed to the guest using the vring, are directly queued in
> a per-socket list. These buffers are preallocated by the guest
> with a fixed size (4 KB).
>
> The maximum amount of memory used by each socket should be
> controlled by the credit mechanism.
> The default credit available per-socket is 256 KB, but if we use
> only 1 byte per packet, the guest can queue up to 262144 of 4 KB
> buffers, using up to 1 GB of memory per-socket. In addition, the
> guest will continue to fill the vring with new 4 KB free buffers
> to avoid starvation of other sockets.
>
> This patch mitigates this issue copying the payload of small
> packets (< 128 bytes) into the buffer of last packet queued, in
> order to avoid wasting memory.
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
This is good enough for net-next, but for net I think we
should figure out how to address the issue completely.
Can we make the accounting precise? What happens to
performance if we do?
> ---
> drivers/vhost/vsock.c | 2 +
> include/linux/virtio_vsock.h | 1 +
> net/vmw_vsock/virtio_transport.c | 1 +
> net/vmw_vsock/virtio_transport_common.c | 60 +++++++++++++++++++++----
> 4 files changed, 55 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index 6a50e1d0529c..6c8390a2af52 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -329,6 +329,8 @@ vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq,
> return NULL;
> }
>
> + pkt->buf_len = pkt->len;
> +
> nbytes = copy_from_iter(pkt->buf, pkt->len, &iov_iter);
> if (nbytes != pkt->len) {
> vq_err(vq, "Expected %u byte payload, got %zu bytes\n",
> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> index e223e2632edd..7d973903f52e 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -52,6 +52,7 @@ struct virtio_vsock_pkt {
> /* socket refcnt not held, only use for cancellation */
> struct vsock_sock *vsk;
> void *buf;
> + u32 buf_len;
> u32 len;
> u32 off;
> bool reply;
> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> index 0815d1357861..082a30936690 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -307,6 +307,7 @@ static void virtio_vsock_rx_fill(struct virtio_vsock *vsock)
> break;
> }
>
> + pkt->buf_len = buf_len;
> pkt->len = buf_len;
>
> sg_init_one(&hdr, &pkt->hdr, sizeof(pkt->hdr));
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 6f1a8aff65c5..095221f94786 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -26,6 +26,9 @@
> /* How long to wait for graceful shutdown of a connection */
> #define VSOCK_CLOSE_TIMEOUT (8 * HZ)
>
> +/* Threshold for detecting small packets to copy */
> +#define GOOD_COPY_LEN 128
> +
> static const struct virtio_transport *virtio_transport_get_ops(void)
> {
> const struct vsock_transport *t = vsock_core_get_transport();
> @@ -64,6 +67,9 @@ virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info *info,
> pkt->buf = kmalloc(len, GFP_KERNEL);
> if (!pkt->buf)
> goto out_pkt;
> +
> + pkt->buf_len = len;
> +
> err = memcpy_from_msg(pkt->buf, info->msg, len);
> if (err)
> goto out;
> @@ -841,24 +847,60 @@ virtio_transport_recv_connecting(struct sock *sk,
> return err;
> }
>
> +static void
> +virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> + struct virtio_vsock_pkt *pkt)
> +{
> + struct virtio_vsock_sock *vvs = vsk->trans;
> + bool free_pkt = false;
> +
> + pkt->len = le32_to_cpu(pkt->hdr.len);
> + pkt->off = 0;
> +
> + spin_lock_bh(&vvs->rx_lock);
> +
> + virtio_transport_inc_rx_pkt(vvs, pkt);
> +
> + /* Try to copy small packets into the buffer of last packet queued,
> + * to avoid wasting memory queueing the entire buffer with a small
> + * payload.
> + */
> + if (pkt->len <= GOOD_COPY_LEN && !list_empty(&vvs->rx_queue)) {
> + struct virtio_vsock_pkt *last_pkt;
> +
> + last_pkt = list_last_entry(&vvs->rx_queue,
> + struct virtio_vsock_pkt, list);
> +
> + /* If there is space in the last packet queued, we copy the
> + * new packet in its buffer.
> + */
> + if (pkt->len <= last_pkt->buf_len - last_pkt->len) {
> + memcpy(last_pkt->buf + last_pkt->len, pkt->buf,
> + pkt->len);
> + last_pkt->len += pkt->len;
> + free_pkt = true;
> + goto out;
> + }
> + }
> +
> + list_add_tail(&pkt->list, &vvs->rx_queue);
> +
> +out:
> + spin_unlock_bh(&vvs->rx_lock);
> + if (free_pkt)
> + virtio_transport_free_pkt(pkt);
> +}
> +
> static int
> virtio_transport_recv_connected(struct sock *sk,
> struct virtio_vsock_pkt *pkt)
> {
> struct vsock_sock *vsk = vsock_sk(sk);
> - struct virtio_vsock_sock *vvs = vsk->trans;
> int err = 0;
>
> switch (le16_to_cpu(pkt->hdr.op)) {
> case VIRTIO_VSOCK_OP_RW:
> - pkt->len = le32_to_cpu(pkt->hdr.len);
> - pkt->off = 0;
> -
> - spin_lock_bh(&vvs->rx_lock);
> - virtio_transport_inc_rx_pkt(vvs, pkt);
> - list_add_tail(&pkt->list, &vvs->rx_queue);
> - spin_unlock_bh(&vvs->rx_lock);
> -
> + virtio_transport_recv_enqueue(vsk, pkt);
> sk->sk_data_ready(sk);
> return err;
> case VIRTIO_VSOCK_OP_CREDIT_UPDATE:
> --
> 2.20.1
^ permalink raw reply
* Re: [PATCH v4 0/5] vsock/virtio: optimizations to increase the throughput
From: Michael S. Tsirkin @ 2019-07-29 13:59 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, linux-kernel, Stefan Hajnoczi, David S. Miller,
virtualization, Jason Wang, kvm
In-Reply-To: <20190717113030.163499-1-sgarzare@redhat.com>
On Wed, Jul 17, 2019 at 01:30:25PM +0200, Stefano Garzarella wrote:
> This series tries to increase the throughput of virtio-vsock with slight
> changes.
> While I was testing the v2 of this series I discovered an huge use of memory,
> so I added patch 1 to mitigate this issue. I put it in this series in order
> to better track the performance trends.
Series:
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Can this go into net-next?
> v4:
> - rebased all patches on current master (conflicts is Patch 4)
> - Patch 1: added Stefan's R-b
> - Patch 3: removed lock when buf_alloc is written [David];
> moved this patch after "vsock/virtio: reduce credit update messages"
> to make it clearer
> - Patch 4: vhost_exceeds_weight() is recently introduced, so I've solved some
> conflicts
>
> v3: https://patchwork.kernel.org/cover/10970145
>
> v2: https://patchwork.kernel.org/cover/10938743
>
> v1: https://patchwork.kernel.org/cover/10885431
>
> Below are the benchmarks step by step. I used iperf3 [1] modified with VSOCK
> support. As Micheal suggested in the v1, I booted host and guest with 'nosmap'.
>
> A brief description of patches:
> - Patches 1: limit the memory usage with an extra copy for small packets
> - Patches 2+3: reduce the number of credit update messages sent to the
> transmitter
> - Patches 4+5: allow the host to split packets on multiple buffers and use
> VIRTIO_VSOCK_MAX_PKT_BUF_SIZE as the max packet size allowed
>
> host -> guest [Gbps]
> pkt_size before opt p 1 p 2+3 p 4+5
>
> 32 0.032 0.030 0.048 0.051
> 64 0.061 0.059 0.108 0.117
> 128 0.122 0.112 0.227 0.234
> 256 0.244 0.241 0.418 0.415
> 512 0.459 0.466 0.847 0.865
> 1K 0.927 0.919 1.657 1.641
> 2K 1.884 1.813 3.262 3.269
> 4K 3.378 3.326 6.044 6.195
> 8K 5.637 5.676 10.141 11.287
> 16K 8.250 8.402 15.976 16.736
> 32K 13.327 13.204 19.013 20.515
> 64K 21.241 21.341 20.973 21.879
> 128K 21.851 22.354 21.816 23.203
> 256K 21.408 21.693 21.846 24.088
> 512K 21.600 21.899 21.921 24.106
>
> guest -> host [Gbps]
> pkt_size before opt p 1 p 2+3 p 4+5
>
> 32 0.045 0.046 0.057 0.057
> 64 0.089 0.091 0.103 0.104
> 128 0.170 0.179 0.192 0.200
> 256 0.364 0.351 0.361 0.379
> 512 0.709 0.699 0.731 0.790
> 1K 1.399 1.407 1.395 1.427
> 2K 2.670 2.684 2.745 2.835
> 4K 5.171 5.199 5.305 5.451
> 8K 8.442 8.500 10.083 9.941
> 16K 12.305 12.259 13.519 15.385
> 32K 11.418 11.150 11.988 24.680
> 64K 10.778 10.659 11.589 35.273
> 128K 10.421 10.339 10.939 40.338
> 256K 10.300 9.719 10.508 36.562
> 512K 9.833 9.808 10.612 35.979
>
> As Stefan suggested in the v1, I measured also the efficiency in this way:
> efficiency = Mbps / (%CPU_Host + %CPU_Guest)
>
> The '%CPU_Guest' is taken inside the VM. I know that it is not the best way,
> but it's provided for free from iperf3 and could be an indication.
>
> host -> guest efficiency [Mbps / (%CPU_Host + %CPU_Guest)]
> pkt_size before opt p 1 p 2+3 p 4+5
>
> 32 0.35 0.45 0.79 1.02
> 64 0.56 0.80 1.41 1.54
> 128 1.11 1.52 3.03 3.12
> 256 2.20 2.16 5.44 5.58
> 512 4.17 4.18 10.96 11.46
> 1K 8.30 8.26 20.99 20.89
> 2K 16.82 16.31 39.76 39.73
> 4K 30.89 30.79 74.07 75.73
> 8K 53.74 54.49 124.24 148.91
> 16K 80.68 83.63 200.21 232.79
> 32K 132.27 132.52 260.81 357.07
> 64K 229.82 230.40 300.19 444.18
> 128K 332.60 329.78 331.51 492.28
> 256K 331.06 337.22 339.59 511.59
> 512K 335.58 328.50 331.56 504.56
>
> guest -> host efficiency [Mbps / (%CPU_Host + %CPU_Guest)]
> pkt_size before opt p 1 p 2+3 p 4+5
>
> 32 0.43 0.43 0.53 0.56
> 64 0.85 0.86 1.04 1.10
> 128 1.63 1.71 2.07 2.13
> 256 3.48 3.35 4.02 4.22
> 512 6.80 6.67 7.97 8.63
> 1K 13.32 13.31 15.72 15.94
> 2K 25.79 25.92 30.84 30.98
> 4K 50.37 50.48 58.79 59.69
> 8K 95.90 96.15 107.04 110.33
> 16K 145.80 145.43 143.97 174.70
> 32K 147.06 144.74 146.02 282.48
> 64K 145.25 143.99 141.62 406.40
> 128K 149.34 146.96 147.49 489.34
> 256K 156.35 149.81 152.21 536.37
> 512K 151.65 150.74 151.52 519.93
>
> [1] https://github.com/stefano-garzarella/iperf/
>
> Stefano Garzarella (5):
> vsock/virtio: limit the memory used per-socket
> vsock/virtio: reduce credit update messages
> vsock/virtio: fix locking in virtio_transport_inc_tx_pkt()
> vhost/vsock: split packets to send using multiple buffers
> vsock/virtio: change the maximum packet size allowed
>
> drivers/vhost/vsock.c | 68 ++++++++++++-----
> include/linux/virtio_vsock.h | 4 +-
> net/vmw_vsock/virtio_transport.c | 1 +
> net/vmw_vsock/virtio_transport_common.c | 99 ++++++++++++++++++++-----
> 4 files changed, 134 insertions(+), 38 deletions(-)
>
> --
> 2.20.1
^ permalink raw reply
* Re: [PATCH v6 rdma-next 1/6] RDMA/core: Create mmap database and cookie helper functions
From: Gal Pressman @ 2019-07-29 13:53 UTC (permalink / raw)
To: Michal Kalderon, Jason Gunthorpe
Cc: Ariel Elior, dledford@redhat.com, linux-rdma@vger.kernel.org,
davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <MN2PR18MB3182F4557BC042EE37A3C565A1DD0@MN2PR18MB3182.namprd18.prod.outlook.com>
On 29/07/2019 15:58, Michal Kalderon wrote:
>> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
>> owner@vger.kernel.org> On Behalf Of Jason Gunthorpe
>>
>>> + xa_lock(&ucontext->mmap_xa);
>>> + if (check_add_overflow(ucontext->mmap_xa_page,
>>> + (u32)(length >> PAGE_SHIFT),
>>> + &next_mmap_page))
>>> + goto err_unlock;
>>
>> I still don't like that this algorithm latches into a permanent failure when the
>> xa_page wraps.
>>
>> It seems worth spending a bit more time here to tidy this.. Keep using the
>> mmap_xa_page scheme, but instead do something like
>>
>> alloc_cyclic_range():
>>
>> while () {
>> // Find first empty element in a cyclic way
>> xa_page_first = mmap_xa_page;
>> xa_find(xa, &xa_page_first, U32_MAX, XA_FREE_MARK)
>>
>> // Is there a enough room to have the range?
>> if (check_add_overflow(xa_page_first, npages, &xa_page_end)) {
>> mmap_xa_page = 0;
>> continue;
>> }
>>
>> // See if the element before intersects
>> elm = xa_find(xa, &zero, xa_page_end, 0);
>> if (elm && intersects(xa_page_first, xa_page_last, elm->first, elm->last)) {
>> mmap_xa_page = elm->last + 1;
>> continue
>> }
>>
>> // xa_page_first -> xa_page_end should now be free
>> xa_insert(xa, xa_page_start, entry);
>> mmap_xa_page = xa_page_end + 1;
>> return xa_page_start;
>> }
>>
>> Approximately, please check it.
> Gal & Jason,
>
> Coming back to the mmap_xa_page algorithm. I couldn't find some background on this.
> Why do you need the length to be represented in the mmap_xa_page ?
> Why not simply use xa_alloc_cyclic ( like in siw )
> This is simply a key to a mmap object...
The intention was that the entry would "occupy" number of xarray elements
according to its size (in pages). It wasn't initially like this, but IIRC this
was preferred by Jason.
^ permalink raw reply
* Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
From: Allan W. Nielsen @ 2019-07-29 13:52 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Horatiu Vultur, roopa, davem, bridge, netdev, linux-kernel
In-Reply-To: <3cc69103-d194-2eca-e7dd-e2fa6a730223@cumulusnetworks.com>
The 07/29/2019 15:50, Nikolay Aleksandrov wrote:
> On 29/07/2019 15:22, Nikolay Aleksandrov wrote:
> > Hi Allan,
> > On 29/07/2019 15:14, Allan W. Nielsen wrote:
> >> First of all, as mentioned further down in this thread, I realized that our
> >> implementation of the multicast floodmasks does not align with the existing SW
> >> implementation. We will change this, such that all multicast packets goes to the
> >> SW bridge.
> >>
> >> This changes things a bit, not that much.
> >>
> >> I actually think you summarized the issue we have (after changing to multicast
> >> flood-masks) right here:
> >>
> >> The 07/26/2019 12:26, Nikolay Aleksandrov wrote:
> >>>>> Actually you mentioned non-IP traffic, so the querier stuff is not a problem. This
> >>>>> traffic will always be flooded by the bridge (and also a copy will be locally sent up).
> >>>>> Thus only the flooding may need to be controlled.
> >>
> >> This seems to be exactly what we need.
> >>
> >> Assuming we have a SW bridge (br0) with 4 slave interfaces (eth0-3). We use this
> >> on a network where we want to limit the flooding of frames with dmac
> >> 01:21:6C:00:00:01 (which is non IP traffic) to eth0 and eth1.
> >>
> >> One way of doing this could potentially be to support the following command:
> >>
> >> bridge fdb add 01:21:6C:00:00:01 port eth0
> >> bridge fdb append 01:21:6C:00:00:01 port eth1
> >>
>
> And the fdbs become linked lists?
Yes, it will most likely become a linked list
> So we'll increase the complexity for something that is already supported by
> ACLs (e.g. tc) and also bridge per-port multicast flood flag ?
I do not think it can be supported with the facilities we have today in tc.
We can do half of it (copy more fraems to the CPU) with tc, but we can not limit
the floodmask of a frame with tc (say we want it to flood to 2 out of 4 slave
ports).
> I'm sorry but that doesn't sound good to me for a case which is very rare and
> there are existing ways to solve without incurring performance hits or increasing
> code complexity.
I do not consider it rarely, controling the forwarding of L2 multicast frames is
quite common in the applications we are doing.
> If you find a way to achieve this without incurring a performance hit or significant
> code complexity increase, and without breaking current use-cases (e.g. unexpected default
> forwarding behaviour changes) then please send a patch and we can discuss it further with
> all details present. People have provided enough alternatives which avoid all of the
> problems.
Will do, thanks for the guidance.
/Allan
^ permalink raw reply
* Re: [PATCH] net: phy: phy_led_triggers: Fix a possible null-pointer dereference in phy_led_trigger_change_speed()
From: Andrew Lunn @ 2019-07-29 13:45 UTC (permalink / raw)
To: Jia-Ju Bai; +Cc: f.fainelli, hkallweit1, davem, netdev, linux-kernel
In-Reply-To: <20190729092424.30928-1-baijiaju1990@gmail.com>
On Mon, Jul 29, 2019 at 05:24:24PM +0800, Jia-Ju Bai wrote:
> In phy_led_trigger_change_speed(), there is an if statement on line 48
> to check whether phy->last_triggered is NULL:
> if (!phy->last_triggered)
>
> When phy->last_triggered is NULL, it is used on line 52:
> led_trigger_event(&phy->last_triggered->trigger, LED_OFF);
>
> Thus, a possible null-pointer dereference may occur.
>
> To fix this bug, led_trigger_event(&phy->last_triggered->trigger,
> LED_OFF) is called when phy->last_triggered is not NULL.
>
> This bug is found by a static analysis tool STCheck written by us.
Who is 'us'?
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
From: Nikolay Aleksandrov @ 2019-07-29 13:42 UTC (permalink / raw)
To: Allan W. Nielsen
Cc: Horatiu Vultur, roopa, davem, bridge, netdev, linux-kernel
In-Reply-To: <20190729131420.tqukz55tz26jkg73@lx-anielsen.microsemi.net>
On 29/07/2019 16:14, Allan W. Nielsen wrote:
> The 07/29/2019 15:22, Nikolay Aleksandrov wrote:
>> Yes, all of the multicast code is handled differently, it doesn't go through the fdb
>> lookup or code at all. I don't see how you'll do a lookup in the fdb table with a
>> multicast mac address, take a look at br_handle_frame_finish() and you'll notice
>> that when a multicast dmac is detected then we use the bridge mcast code for lookups
>> and forwarding.
>
> Here is my thinking (needs much more elaboration, which will come if we do a
> patch to test it out):
>
>
> In br_pkt_type
>
> Rename BR_PKT_MULTICAST to BR_PKT_MULTICAST_IP
> Add a new type called BR_PKT_MULTICAST_L2
>
> In br_handle_frame_finish
>
> if (is_multicast_ether_addr(dest)) {
> /* by definition the broadcast is also a multicast address */
> if (is_broadcast_ether_addr(dest)) {
> pkt_type = BR_PKT_BROADCAST;
> local_rcv = true;
> } else {
> pkt_type = BR_PKT_MULTICAST;
> if (br_multicast_rcv(br, p, skb, vid))
> goto drop;
> }
> }
>
> Change the code above to detect if it is a BR_PKT_MULTICAST_IP or a
> BR_PKT_MULTICAST_L2
>
>
> In this section:
>
> switch (pkt_type) {
> ....
> }
>
> if (dst) {
> } else {
> }
>
> Add awareness to the BR_PKT_MULTICAST_L2 type, and allow it do forwarding
> according to the static entry if it is there.
>
All of these add overhead to everyone - standard unicast and multicast forwarding.
Also as mentioned in my second email the fdb would need changes which would further
increase that overhead and also the code complexity for everyone for a use-case which
is very rare when there are alternatives today which avoid all of that. Offload tc rules
and add a rule to allow that traffic on ingress for particular ports, then either use
the bridge multicast flood flag or tc again to restrict flood to other egress ports.
Alternatively use ip maddr add which calls dev_mc_add() which is what the patch was
trying to do in the first place to allow the Rx of these packets and then control
the flooding via one of the above methods.
Alternatively offload nft and use that to control the traffic.
I'm sure there are more ways that I'm missing. :)
If you find a way to achieve this without incurring a performance hit or significant
code complexity increase, and without breaking current use-cases (e.g. unexpected default
forwarding behaviour changes) then please send a patch and we can discuss it further with
all details present. People have provided enough alternatives which avoid all of the
problems.
>> If you're trying to achieve Rx only on the bridge of these then
>> why not just use Ido's tc suggestion or even the ip maddr add offload for each port ?
>>
>> If you add a multicast mac in the fdb (currently allowed, but has no effect) and you
>> use dev_mc_add() as suggested that'd just be a hack to pass it down and it is already
>> possible to achieve via other methods, no need to go through the bridge.
>
> Well, I wanted the SW bridge implementation to behave the same with an without
> HW offload.
>
> And also, I believe that is conceptually belongs to the MAC tables.
>
> /Allan
>
^ permalink raw reply
* [PATCH v4 12/14] NFC: nxp-nci: Remove 'default n' for the core
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
Sedat Dilek
Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>
It seems contributors follow the style of Kconfig entries where explicit
'default n' is present. The default 'default' is 'n' already, thus, drop
these lines from Kconfig to make it more clear.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
drivers/nfc/nxp-nci/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/nfc/nxp-nci/Kconfig b/drivers/nfc/nxp-nci/Kconfig
index 12df2c8cc51d..ed6cbdf0f0b4 100644
--- a/drivers/nfc/nxp-nci/Kconfig
+++ b/drivers/nfc/nxp-nci/Kconfig
@@ -2,7 +2,6 @@
config NFC_NXP_NCI
tristate "NXP-NCI NFC driver"
depends on NFC_NCI
- default n
---help---
Generic core driver for NXP NCI chips such as the NPC100
or PN7150 families.
--
2.20.1
^ permalink raw reply related
* [PATCH v4 10/14] NFC: nxp-nci: Drop comma in terminator lines
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
Sedat Dilek
Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>
There is no need to have a comma after terminator entry
in the arrays of IDs.
This may prevent the misguided addition behind the terminator
without compiler notice.
Drop the comma in terminator lines for good.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
drivers/nfc/nxp-nci/i2c.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index f2c8a560e265..59b0a02a813d 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -325,7 +325,7 @@ MODULE_DEVICE_TABLE(i2c, nxp_nci_i2c_id_table);
static const struct of_device_id of_nxp_nci_i2c_match[] = {
{ .compatible = "nxp,nxp-nci-i2c", },
- {},
+ {}
};
MODULE_DEVICE_TABLE(of, of_nxp_nci_i2c_match);
@@ -333,7 +333,7 @@ MODULE_DEVICE_TABLE(of, of_nxp_nci_i2c_match);
static const struct acpi_device_id acpi_id[] = {
{ "NXP1001" },
{ "NXP7471" },
- { },
+ { }
};
MODULE_DEVICE_TABLE(acpi, acpi_id);
#endif
--
2.20.1
^ permalink raw reply related
* [PATCH v4 00/14] NFC: nxp-nci: clean up and new device support
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
Sedat Dilek
Cc: Andy Shevchenko
Few people reported that some laptops are coming with new ACPI ID for the
devices should be supported by nxp-nci driver.
This series adds new ID (patch 2), cleans up the driver from legacy platform
data and unifies GPIO request for Device Tree and ACPI (patches 3-6), removes
dead or unneeded code (patches 7, 9, 11), constifies ID table (patch 8),
removes comma in terminator line for better maintenance (patch 10) and
rectifies Kconfig entry (patches 12-14).
It also contains a fix for NFC subsystem as suggested by Sedat.
Series has been tested by Sedat.
Changelog v4:
- rebased on top of latest linux-next
- appended cover letter
- elaborated removal of pr_fmt() in the patch 11 (David)
Andrey Konovalov (1):
NFC: fix attrs checks in netlink interface
Andy Shevchenko (11):
NFC: nxp-nci: Add NXP1001 to the ACPI ID table
NFC: nxp-nci: Get rid of platform data
NFC: nxp-nci: Convert to use GPIO descriptor
NFC: nxp-nci: Add GPIO ACPI mapping table
NFC: nxp-nci: Get rid of code duplication in ->probe()
NFC: nxp-nci: Get rid of useless label
NFC: nxp-nci: Constify acpi_device_id
NFC: nxp-nci: Drop of_match_ptr() use
NFC: nxp-nci: Drop comma in terminator lines
NFC: nxp-nci: Remove unused macro pr_fmt()
NFC: nxp-nci: Remove 'default n' for the core
Sedat Dilek (2):
NFC: nxp-nci: Clarify on supported chips
NFC: nxp-nci: Fix recommendation for NFC_NXP_NCI_I2C Kconfig
MAINTAINERS | 1 -
drivers/nfc/nxp-nci/Kconfig | 7 +-
drivers/nfc/nxp-nci/core.c | 2 -
drivers/nfc/nxp-nci/i2c.c | 134 +++++++-------------------
drivers/nfc/nxp-nci/nxp-nci.h | 1 -
include/linux/platform_data/nxp-nci.h | 19 ----
net/nfc/netlink.c | 6 +-
7 files changed, 41 insertions(+), 129 deletions(-)
delete mode 100644 include/linux/platform_data/nxp-nci.h
--
2.20.1
^ permalink raw reply
* [PATCH v4 09/14] NFC: nxp-nci: Drop of_match_ptr() use
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
Sedat Dilek
Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>
There is no need to guard OF device ID table with of_match_ptr().
Otherwise we would get a defined but not used data.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
drivers/nfc/nxp-nci/i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 4e71962dc557..f2c8a560e265 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -342,7 +342,7 @@ static struct i2c_driver nxp_nci_i2c_driver = {
.driver = {
.name = NXP_NCI_I2C_DRIVER_NAME,
.acpi_match_table = ACPI_PTR(acpi_id),
- .of_match_table = of_match_ptr(of_nxp_nci_i2c_match),
+ .of_match_table = of_nxp_nci_i2c_match,
},
.probe = nxp_nci_i2c_probe,
.id_table = nxp_nci_i2c_id_table,
--
2.20.1
^ permalink raw reply related
* [PATCH v4 02/14] NFC: nxp-nci: Add NXP1001 to the ACPI ID table
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
Sedat Dilek
Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>
It seems a lot of laptops are equipped with NXP NFC300 chip with
the ACPI ID NXP1001 as per DSDT.
Append it to the driver's ACPI ID table.
Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
drivers/nfc/nxp-nci/i2c.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 4aeb3861b409..5db71869f04b 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -396,6 +396,7 @@ MODULE_DEVICE_TABLE(of, of_nxp_nci_i2c_match);
#ifdef CONFIG_ACPI
static struct acpi_device_id acpi_id[] = {
+ { "NXP1001" },
{ "NXP7471" },
{ },
};
--
2.20.1
^ permalink raw reply related
* [PATCH v4 04/14] NFC: nxp-nci: Convert to use GPIO descriptor
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
Sedat Dilek
Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>
Since we got rid of platform data, the driver may use
GPIO descriptor directly.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
drivers/nfc/nxp-nci/core.c | 1 -
drivers/nfc/nxp-nci/i2c.c | 60 ++++++++++----------------------------
2 files changed, 15 insertions(+), 46 deletions(-)
diff --git a/drivers/nfc/nxp-nci/core.c b/drivers/nfc/nxp-nci/core.c
index aed18ca60170..a0ce95a287c5 100644
--- a/drivers/nfc/nxp-nci/core.c
+++ b/drivers/nfc/nxp-nci/core.c
@@ -11,7 +11,6 @@
*/
#include <linux/delay.h>
-#include <linux/gpio.h>
#include <linux/module.h>
#include <linux/nfc.h>
diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 47b3b7e612e6..713c267acf88 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -21,8 +21,6 @@
#include <linux/module.h>
#include <linux/nfc.h>
#include <linux/gpio/consumer.h>
-#include <linux/of_gpio.h>
-#include <linux/of_irq.h>
#include <asm/unaligned.h>
#include <net/nfc/nfc.h>
@@ -37,8 +35,8 @@ struct nxp_nci_i2c_phy {
struct i2c_client *i2c_dev;
struct nci_dev *ndev;
- unsigned int gpio_en;
- unsigned int gpio_fw;
+ struct gpio_desc *gpiod_en;
+ struct gpio_desc *gpiod_fw;
int hard_fault; /*
* < 0 if hardware error occurred (e.g. i2c err)
@@ -51,8 +49,8 @@ static int nxp_nci_i2c_set_mode(void *phy_id,
{
struct nxp_nci_i2c_phy *phy = (struct nxp_nci_i2c_phy *) phy_id;
- gpio_set_value(phy->gpio_fw, (mode == NXP_NCI_MODE_FW) ? 1 : 0);
- gpio_set_value(phy->gpio_en, (mode != NXP_NCI_MODE_COLD) ? 1 : 0);
+ gpiod_set_value(phy->gpiod_fw, (mode == NXP_NCI_MODE_FW) ? 1 : 0);
+ gpiod_set_value(phy->gpiod_en, (mode != NXP_NCI_MODE_COLD) ? 1 : 0);
usleep_range(10000, 15000);
if (mode == NXP_NCI_MODE_COLD)
@@ -252,30 +250,18 @@ static irqreturn_t nxp_nci_i2c_irq_thread_fn(int irq, void *phy_id)
static int nxp_nci_i2c_parse_devtree(struct i2c_client *client)
{
struct nxp_nci_i2c_phy *phy = i2c_get_clientdata(client);
- struct device_node *pp;
- int r;
-
- pp = client->dev.of_node;
- if (!pp)
- return -ENODEV;
- r = of_get_named_gpio(pp, "enable-gpios", 0);
- if (r == -EPROBE_DEFER)
- r = of_get_named_gpio(pp, "enable-gpios", 0);
- if (r < 0) {
- nfc_err(&client->dev, "Failed to get EN gpio, error: %d\n", r);
- return r;
+ phy->gpiod_en = devm_gpiod_get(&client->dev, "enable", GPIOD_OUT_LOW);
+ if (IS_ERR(phy->gpiod_en)) {
+ nfc_err(&client->dev, "Failed to get EN gpio\n");
+ return PTR_ERR(phy->gpiod_en);
}
- phy->gpio_en = r;
- r = of_get_named_gpio(pp, "firmware-gpios", 0);
- if (r == -EPROBE_DEFER)
- r = of_get_named_gpio(pp, "firmware-gpios", 0);
- if (r < 0) {
- nfc_err(&client->dev, "Failed to get FW gpio, error: %d\n", r);
- return r;
+ phy->gpiod_fw = devm_gpiod_get(&client->dev, "firmware", GPIOD_OUT_LOW);
+ if (IS_ERR(phy->gpiod_fw)) {
+ nfc_err(&client->dev, "Failed to get FW gpio\n");
+ return PTR_ERR(phy->gpiod_fw);
}
- phy->gpio_fw = r;
return 0;
}
@@ -283,19 +269,15 @@ static int nxp_nci_i2c_parse_devtree(struct i2c_client *client)
static int nxp_nci_i2c_acpi_config(struct nxp_nci_i2c_phy *phy)
{
struct i2c_client *client = phy->i2c_dev;
- struct gpio_desc *gpiod_en, *gpiod_fw;
- gpiod_en = devm_gpiod_get_index(&client->dev, NULL, 2, GPIOD_OUT_LOW);
- gpiod_fw = devm_gpiod_get_index(&client->dev, NULL, 1, GPIOD_OUT_LOW);
+ phy->gpiod_en = devm_gpiod_get_index(&client->dev, NULL, 2, GPIOD_OUT_LOW);
+ phy->gpiod_fw = devm_gpiod_get_index(&client->dev, NULL, 1, GPIOD_OUT_LOW);
- if (IS_ERR(gpiod_en) || IS_ERR(gpiod_fw)) {
+ if (IS_ERR(phy->gpiod_en) || IS_ERR(phy->gpiod_fw)) {
nfc_err(&client->dev, "No GPIOs\n");
return -EINVAL;
}
- phy->gpio_en = desc_to_gpio(gpiod_en);
- phy->gpio_fw = desc_to_gpio(gpiod_fw);
-
return 0;
}
@@ -331,24 +313,12 @@ static int nxp_nci_i2c_probe(struct i2c_client *client,
r = nxp_nci_i2c_acpi_config(phy);
if (r < 0)
goto probe_exit;
- goto nci_probe;
} else {
nfc_err(&client->dev, "No platform data\n");
r = -EINVAL;
goto probe_exit;
}
- r = devm_gpio_request_one(&phy->i2c_dev->dev, phy->gpio_en,
- GPIOF_OUT_INIT_LOW, "nxp_nci_en");
- if (r < 0)
- goto probe_exit;
-
- r = devm_gpio_request_one(&phy->i2c_dev->dev, phy->gpio_fw,
- GPIOF_OUT_INIT_LOW, "nxp_nci_fw");
- if (r < 0)
- goto probe_exit;
-
-nci_probe:
r = nxp_nci_probe(phy, &client->dev, &i2c_phy_ops,
NXP_NCI_I2C_MAX_PAYLOAD, &phy->ndev);
if (r < 0)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 2/4] net: phy: Add mdio-aspeed
From: Andrew Lunn @ 2019-07-29 13:32 UTC (permalink / raw)
To: Andrew Jeffery
Cc: netdev, davem, robh+dt, mark.rutland, joel, f.fainelli,
hkallweit1, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel
In-Reply-To: <20190729043926.32679-3-andrew@aj.id.au>
On Mon, Jul 29, 2019 at 02:09:24PM +0930, Andrew Jeffery wrote:
> The AST2600 design separates the MDIO controllers from the MAC, which is
> where they were placed in the AST2400 and AST2500. Further, the register
> interface is reworked again, so now we have three possible different
> interface implementations, however this driver only supports the
> interface provided by the AST2600. The AST2400 and AST2500 will continue
> to be supported by the MDIO support embedded in the FTGMAC100 driver.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> drivers/net/phy/Kconfig | 13 +++
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/mdio-aspeed.c | 159 ++++++++++++++++++++++++++++++++++
> 3 files changed, 173 insertions(+)
> create mode 100644 drivers/net/phy/mdio-aspeed.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 20f14c5fbb7e..206d8650ee7f 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -21,6 +21,19 @@ config MDIO_BUS
>
> if MDIO_BUS
>
> +config MDIO_ASPEED
> + tristate "ASPEED MDIO bus controller"
> + depends on ARCH_ASPEED || COMPILE_TEST
> + depends on OF_MDIO && HAS_IOMEM
> + help
> + This module provides a driver for the independent MDIO bus
> + controllers found in the ASPEED AST2600 SoC. This is a driver for the
> + third revision of the ASPEED MDIO register interface - the first two
> + revisions are the "old" and "new" interfaces found in the AST2400 and
> + AST2500, embedded in the MAC. For legacy reasons, FTGMAC100 driver
> + continues to drive the embedded MDIO controller for the AST2400 and
> + AST2500 SoCs, so say N if AST2600 support is not required.
> +
> config MDIO_BCM_IPROC
> tristate "Broadcom iProc MDIO bus controller"
> depends on ARCH_BCM_IPROC || COMPILE_TEST
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 839acb292c38..ba07c27e4208 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -22,6 +22,7 @@ libphy-$(CONFIG_LED_TRIGGER_PHY) += phy_led_triggers.o
> obj-$(CONFIG_PHYLINK) += phylink.o
> obj-$(CONFIG_PHYLIB) += libphy.o
>
> +obj-$(CONFIG_MDIO_ASPEED) += mdio-aspeed.o
> obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
> obj-$(CONFIG_MDIO_BCM_UNIMAC) += mdio-bcm-unimac.o
> obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o
> diff --git a/drivers/net/phy/mdio-aspeed.c b/drivers/net/phy/mdio-aspeed.c
> new file mode 100644
> index 000000000000..71496a9ff54a
> --- /dev/null
> +++ b/drivers/net/phy/mdio-aspeed.c
> @@ -0,0 +1,159 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* Copyright (C) 2019 IBM Corp. */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/mdio.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_mdio.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +
> +#define DRV_NAME "mdio-aspeed"
> +
> +#define ASPEED_MDIO_CTRL 0x0
> +#define ASPEED_MDIO_CTRL_FIRE BIT(31)
> +#define ASPEED_MDIO_CTRL_ST BIT(28)
> +#define ASPEED_MDIO_CTRL_ST_C45 0
> +#define ASPEED_MDIO_CTRL_ST_C22 1
> +#define ASPEED_MDIO_CTRL_OP GENMASK(27, 26)
> +#define MDIO_C22_OP_WRITE 0b01
> +#define MDIO_C22_OP_READ 0b10
> +#define ASPEED_MDIO_CTRL_PHYAD GENMASK(25, 21)
> +#define ASPEED_MDIO_CTRL_REGAD GENMASK(20, 16)
> +#define ASPEED_MDIO_CTRL_MIIWDATA GENMASK(15, 0)
> +
> +#define ASPEED_MDIO_DATA 0x4
> +#define ASPEED_MDIO_DATA_MDC_THRES GENMASK(31, 24)
> +#define ASPEED_MDIO_DATA_MDIO_EDGE BIT(23)
> +#define ASPEED_MDIO_DATA_MDIO_LATCH GENMASK(22, 20)
> +#define ASPEED_MDIO_DATA_IDLE BIT(16)
> +#define ASPEED_MDIO_DATA_MIIRDATA GENMASK(15, 0)
> +
> +#define ASPEED_MDIO_RETRIES 10
> +
> +struct aspeed_mdio {
> + void __iomem *base;
> +};
> +
> +static int aspeed_mdio_read(struct mii_bus *bus, int addr, int regnum)
> +{
> + struct aspeed_mdio *ctx = bus->priv;
> + u32 ctrl;
> + int i;
> +
> + dev_dbg(&bus->dev, "%s: addr: %d, regnum: %d\n", __func__, addr,
> + regnum);
> +
> + /* Just clause 22 for the moment */
> + ctrl = ASPEED_MDIO_CTRL_FIRE
Hi Andrew
In the binding, you say C45 is supported. Here you don't. It would be
nice to be consistent.
> + | FIELD_PREP(ASPEED_MDIO_CTRL_ST, ASPEED_MDIO_CTRL_ST_C22)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_OP, MDIO_C22_OP_READ)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_PHYAD, addr)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_REGAD, regnum);
> +
> + iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL);
> +
> + for (i = 0; i < ASPEED_MDIO_RETRIES; i++) {
> + u32 data;
> +
> + data = ioread32(ctx->base + ASPEED_MDIO_DATA);
> + if (data & ASPEED_MDIO_DATA_IDLE)
> + return FIELD_GET(ASPEED_MDIO_DATA_MIIRDATA, data);
> +
> + udelay(100);
> + }
One of the readx_poll_timeout functions could be used.
> +
> + dev_err(&bus->dev, "MDIO read failed\n");
> + return -EIO;
> +}
> +
> +static int aspeed_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
> +{
> + struct aspeed_mdio *ctx = bus->priv;
> + u32 ctrl;
> + int i;
> +
> + dev_dbg(&bus->dev, "%s: addr: %d, regnum: %d, val: 0x%x\n",
> + __func__, addr, regnum, val);
> +
> + /* Just clause 22 for the moment */
> + ctrl = ASPEED_MDIO_CTRL_FIRE
> + | FIELD_PREP(ASPEED_MDIO_CTRL_ST, ASPEED_MDIO_CTRL_ST_C22)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_OP, MDIO_C22_OP_WRITE)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_PHYAD, addr)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_REGAD, regnum)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_MIIWDATA, val);
> +
> + iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL);
> +
> + for (i = 0; i < ASPEED_MDIO_RETRIES; i++) {
> + ctrl = ioread32(ctx->base + ASPEED_MDIO_CTRL);
> + if (!(ctrl & ASPEED_MDIO_CTRL_FIRE))
> + return 0;
> +
> + udelay(100);
> + }
readx_poll_timeout() here as well.
Otherwise this looks good.
Andrew
^ 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