* [PATCH 06/11] PCI: of: Add inbound resource parsing to helpers
From: Rob Herring @ 2019-09-24 21:46 UTC (permalink / raw)
To: linux-pci, Bjorn Helgaas, Lorenzo Pieralisi
Cc: Heiko Stuebner, Karthikeyan Mitran, Linus Walleij,
Thomas Petazzoni, Toan Le, Will Deacon, Ryder Lee, Michal Simek,
linux-rockchip, bcm-kernel-feedback-list, Shawn Lin, Ray Jui,
Hou Zhiqiang, Simon Horman, linux-mediatek, linux-arm-kernel,
Scott Branden, Jingoo Han, rfi, linux-renesas-soc, Tom Joseph,
Gustavo Pimentel, Ley Foon Tan
In-Reply-To: <20190924214630.12817-1-robh@kernel.org>
Extend devm_of_pci_get_host_bridge_resources() and
pci_parse_request_of_pci_ranges() helpers to also parse the inbound
addresses from DT 'dma-ranges' and populate a resource list with the
translated addresses. This will help ensure 'dma-ranges' is always
parsed in a consistent way.
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Will Deacon <will@kernel.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Toan Le <toan@os.amperecomputing.com>
Cc: Ley Foon Tan <lftan@altera.com>
Cc: Tom Joseph <tjoseph@cadence.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: bcm-kernel-feedback-list@broadcom.com
Cc: Ryder Lee <ryder.lee@mediatek.com>
Cc: Karthikeyan Mitran <m.karthikeyan@mobiveil.co.in>
Cc: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
Cc: Simon Horman <horms@verge.net.au>
Cc: Shawn Lin <shawn.lin@rock-chips.com>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: rfi@lists.rocketboards.org
Cc: linux-mediatek@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
.../pci/controller/dwc/pcie-designware-host.c | 3 +-
drivers/pci/controller/pci-aardvark.c | 2 +-
drivers/pci/controller/pci-ftpci100.c | 3 +-
drivers/pci/controller/pci-host-common.c | 2 +-
drivers/pci/controller/pci-v3-semi.c | 2 +-
drivers/pci/controller/pci-versatile.c | 2 +-
drivers/pci/controller/pci-xgene.c | 1 +
drivers/pci/controller/pcie-altera.c | 2 +-
drivers/pci/controller/pcie-cadence-host.c | 2 +-
drivers/pci/controller/pcie-iproc-platform.c | 1 +
drivers/pci/controller/pcie-mediatek.c | 2 +-
drivers/pci/controller/pcie-mobiveil.c | 4 +-
drivers/pci/controller/pcie-rcar.c | 3 +-
drivers/pci/controller/pcie-rockchip-host.c | 3 +-
drivers/pci/controller/pcie-xilinx-nwl.c | 2 +-
drivers/pci/controller/pcie-xilinx.c | 2 +-
drivers/pci/of.c | 44 ++++++++++++++++++-
drivers/pci/pci.h | 8 +++-
include/linux/pci.h | 2 +
19 files changed, 72 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index f93252d0da5b..0743ae64bb0f 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -342,7 +342,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
return -ENOMEM;
ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
- &bridge->windows, &pp->io_base);
+ &bridge->windows, &bridge->dma_ranges,
+ &pp->io_base);
if (ret)
return ret;
diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index ff3af3d34028..c00b391529f3 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -940,7 +940,7 @@ static int advk_pcie_probe(struct platform_device *pdev)
}
ret = pci_parse_request_of_pci_ranges(dev, &pcie->resources,
- &bus);
+ &bridge->dma_ranges, &bus);
if (ret) {
dev_err(dev, "Failed to parse resources\n");
return ret;
diff --git a/drivers/pci/controller/pci-ftpci100.c b/drivers/pci/controller/pci-ftpci100.c
index bf5ece5d9291..3e07a8203736 100644
--- a/drivers/pci/controller/pci-ftpci100.c
+++ b/drivers/pci/controller/pci-ftpci100.c
@@ -481,7 +481,8 @@ static int faraday_pci_probe(struct platform_device *pdev)
return PTR_ERR(p->base);
ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
- &res, &io_base);
+ &res, &host->dma_ranges,
+ &io_base);
if (ret)
return ret;
diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index c742881b5061..183381c19ee8 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -27,7 +27,7 @@ static struct pci_config_window *gen_pci_init(struct device *dev,
struct pci_config_window *cfg;
/* Parse our PCI ranges and request their resources */
- err = pci_parse_request_of_pci_ranges(dev, resources, &bus_range);
+ err = pci_parse_request_of_pci_ranges(dev, resources, NULL, &bus_range);
if (err)
return ERR_PTR(err);
diff --git a/drivers/pci/controller/pci-v3-semi.c b/drivers/pci/controller/pci-v3-semi.c
index d219404bad92..090df766faf9 100644
--- a/drivers/pci/controller/pci-v3-semi.c
+++ b/drivers/pci/controller/pci-v3-semi.c
@@ -794,7 +794,7 @@ static int v3_pci_probe(struct platform_device *pdev)
return PTR_ERR(v3->config_base);
ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
- &io_base);
+ &host->dma_ranges, &io_base);
if (ret)
return ret;
diff --git a/drivers/pci/controller/pci-versatile.c b/drivers/pci/controller/pci-versatile.c
index 237b1abb26f2..211159f54b4a 100644
--- a/drivers/pci/controller/pci-versatile.c
+++ b/drivers/pci/controller/pci-versatile.c
@@ -93,7 +93,7 @@ static int versatile_pci_probe(struct platform_device *pdev)
if (IS_ERR(versatile_cfg_base[1]))
return PTR_ERR(versatile_cfg_base[1]);
- ret = pci_parse_request_of_pci_ranges(dev, &pci_res, NULL);
+ ret = pci_parse_request_of_pci_ranges(dev, &pci_res, NULL, NULL);
if (ret)
return ret;
diff --git a/drivers/pci/controller/pci-xgene.c b/drivers/pci/controller/pci-xgene.c
index ffda3e8b4742..11f27c42c06a 100644
--- a/drivers/pci/controller/pci-xgene.c
+++ b/drivers/pci/controller/pci-xgene.c
@@ -635,6 +635,7 @@ static int xgene_pcie_probe(struct platform_device *pdev)
return ret;
ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
+ &bridge->dma_ranges,
&iobase);
if (ret)
return ret;
diff --git a/drivers/pci/controller/pcie-altera.c b/drivers/pci/controller/pcie-altera.c
index 2ed00babff5a..fde6a3b16fb9 100644
--- a/drivers/pci/controller/pcie-altera.c
+++ b/drivers/pci/controller/pcie-altera.c
@@ -801,7 +801,7 @@ static int altera_pcie_probe(struct platform_device *pdev)
}
ret = pci_parse_request_of_pci_ranges(dev, &pcie->resources,
- NULL);
+ &bridge->dma_ranges, NULL);
if (ret) {
dev_err(dev, "Failed add resources\n");
return ret;
diff --git a/drivers/pci/controller/pcie-cadence-host.c b/drivers/pci/controller/pcie-cadence-host.c
index 97e251090b4f..a8f7a6284c3e 100644
--- a/drivers/pci/controller/pcie-cadence-host.c
+++ b/drivers/pci/controller/pcie-cadence-host.c
@@ -211,7 +211,7 @@ static int cdns_pcie_host_init(struct device *dev,
int err;
/* Parse our PCI ranges and request their resources */
- err = pci_parse_request_of_pci_ranges(dev, resources, &bus_range);
+ err = pci_parse_request_of_pci_ranges(dev, resources, NULL, &bus_range);
if (err)
return err;
diff --git a/drivers/pci/controller/pcie-iproc-platform.c b/drivers/pci/controller/pcie-iproc-platform.c
index 5a3550b6bb29..62eee2d17f43 100644
--- a/drivers/pci/controller/pcie-iproc-platform.c
+++ b/drivers/pci/controller/pcie-iproc-platform.c
@@ -101,6 +101,7 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
}
ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, &resources,
+ &bridge->dma_ranges,
&iobase);
if (ret) {
dev_err(dev, "unable to get PCI host bridge resources\n");
diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index cea4112b5309..f3996eed152d 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -1032,7 +1032,7 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)
int err;
err = pci_parse_request_of_pci_ranges(dev, windows,
- &bus);
+ &host->dma_ranges, &bus);
if (err)
return err;
diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
index 672e633601c7..93064faaa656 100644
--- a/drivers/pci/controller/pcie-mobiveil.c
+++ b/drivers/pci/controller/pcie-mobiveil.c
@@ -875,7 +875,9 @@ static int mobiveil_pcie_probe(struct platform_device *pdev)
/* parse the host bridge base addresses from the device tree file */
ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
- &pcie->resources, &iobase);
+ &pcie->resources,
+ &bridge->dma_ranges,
+ &iobase);
if (ret) {
dev_err(dev, "Getting bridge resources failed\n");
return ret;
diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
index f6a669a9af41..b8d6e86a5539 100644
--- a/drivers/pci/controller/pcie-rcar.c
+++ b/drivers/pci/controller/pcie-rcar.c
@@ -1138,7 +1138,8 @@ static int rcar_pcie_probe(struct platform_device *pdev)
pcie->dev = dev;
platform_set_drvdata(pdev, pcie);
- err = pci_parse_request_of_pci_ranges(dev, &pcie->resources, NULL);
+ err = pci_parse_request_of_pci_ranges(dev, &pcie->resources,
+ &bridge->dma_ranges, NULL);
if (err)
goto err_free_bridge;
diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index 8d20f1793a61..093b816b9dc9 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -996,7 +996,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
goto err_deinit_port;
err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
- &res, &io_base);
+ &res, &bridge->dma_ranges,
+ &io_base);
if (err)
goto err_remove_irq_domain;
diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 45c0f344ccd1..5c9132a02d15 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -846,7 +846,7 @@ static int nwl_pcie_probe(struct platform_device *pdev)
}
err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
- &iobase);
+ &bridge->dma_ranges, &iobase);
if (err) {
dev_err(dev, "Getting bridge resources failed\n");
return err;
diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
index 5bf3af3b28e6..a95faca30291 100644
--- a/drivers/pci/controller/pcie-xilinx.c
+++ b/drivers/pci/controller/pcie-xilinx.c
@@ -648,7 +648,7 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
}
err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
- &iobase);
+ &bridge->dma_ranges, &iobase);
if (err) {
dev_err(dev, "Getting bridge resources failed\n");
return err;
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index bc7b27a28795..8d31e078461f 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -257,7 +257,9 @@ EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
*/
int devm_of_pci_get_host_bridge_resources(struct device *dev,
unsigned char busno, unsigned char bus_max,
- struct list_head *resources, resource_size_t *io_base)
+ struct list_head *resources,
+ struct list_head *ib_resources,
+ resource_size_t *io_base)
{
struct device_node *dev_node = dev->of_node;
struct resource *res, tmp_res;
@@ -340,6 +342,42 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
pci_add_resource_offset(resources, res, res->start - range.pci_addr);
}
+ /* Check for dma-ranges property */
+ if (!ib_resources)
+ return 0;
+ err = of_pci_dma_range_parser_init(&parser, dev_node);
+ if (err)
+ return 0;
+
+ dev_dbg(dev, "Parsing dma-ranges property...\n");
+ for_each_of_pci_range(&parser, &range) {
+ /*
+ * If we failed translation or got a zero-sized region
+ * then skip this range
+ */
+ if (((range.flags & IORESOURCE_TYPE_BITS) != IORESOURCE_MEM) ||
+ range.cpu_addr == OF_BAD_ADDR || range.size == 0)
+ continue;
+
+ dev_info(dev, "IB MEM %#010llx..%#010llx -> %#010llx\n",
+ range.cpu_addr,
+ range.cpu_addr + range.size - 1, range.pci_addr);
+
+
+ err = of_pci_range_to_resource(&range, dev_node, &tmp_res);
+ if (err)
+ continue;
+
+ res = devm_kmemdup(dev, &tmp_res, sizeof(tmp_res), GFP_KERNEL);
+ if (!res) {
+ err = -ENOMEM;
+ goto failed;
+ }
+
+ pci_add_resource_offset(ib_resources, res,
+ res->start - range.pci_addr);
+ }
+
return 0;
failed:
@@ -482,6 +520,7 @@ EXPORT_SYMBOL_GPL(of_irq_parse_and_map_pci);
int pci_parse_request_of_pci_ranges(struct device *dev,
struct list_head *resources,
+ struct list_head *ib_resources,
struct resource **bus_range)
{
int err, res_valid = 0;
@@ -489,8 +528,9 @@ int pci_parse_request_of_pci_ranges(struct device *dev,
struct resource_entry *win, *tmp;
INIT_LIST_HEAD(resources);
+ INIT_LIST_HEAD(ib_resources);
err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, resources,
- &iobase);
+ ib_resources, &iobase);
if (err)
return err;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index d22d1b807701..790458953c54 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -582,11 +582,15 @@ of_pci_get_max_link_speed(struct device_node *node)
#if defined(CONFIG_OF_ADDRESS)
int devm_of_pci_get_host_bridge_resources(struct device *dev,
unsigned char busno, unsigned char bus_max,
- struct list_head *resources, resource_size_t *io_base);
+ struct list_head *resources,
+ struct list_head *ib_resources,
+ resource_size_t *io_base);
#else
static inline int devm_of_pci_get_host_bridge_resources(struct device *dev,
unsigned char busno, unsigned char bus_max,
- struct list_head *resources, resource_size_t *io_base)
+ struct list_head *resources,
+ struct list_head *ib_resources,
+ resource_size_t *io_base);
{
return -EINVAL;
}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 82e4cd1b7ac3..d57389b86f92 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2312,6 +2312,7 @@ void pci_release_bus_of_node(struct pci_bus *bus);
struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus);
int pci_parse_request_of_pci_ranges(struct device *dev,
struct list_head *resources,
+ struct list_head *ib_resources,
struct resource **bus_range);
/* Arch may override this (weak) */
@@ -2326,6 +2327,7 @@ static inline struct irq_domain *
pci_host_bridge_of_msi_domain(struct pci_bus *bus) { return NULL; }
static inline int pci_parse_request_of_pci_ranges(struct device *dev,
struct list_head *resources,
+ struct list_head *ib_resources,
struct resource **bus_range)
{
return -EINVAL;
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 08/11] PCI: v3-semi: Use inbound resources for setup
From: Rob Herring @ 2019-09-24 21:46 UTC (permalink / raw)
To: linux-pci, Bjorn Helgaas, Lorenzo Pieralisi
Cc: Linus Walleij, linux-arm-kernel
In-Reply-To: <20190924214630.12817-1-robh@kernel.org>
Now that the helpers provide the inbound resources in the host bridge
'dma_ranges' resource list, convert the v3-semi host bridge to use
the resource list to setup the inbound addresses.
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/pci/controller/pci-v3-semi.c | 38 ++++++++++++----------------
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/drivers/pci/controller/pci-v3-semi.c b/drivers/pci/controller/pci-v3-semi.c
index 090df766faf9..122b695da591 100644
--- a/drivers/pci/controller/pci-v3-semi.c
+++ b/drivers/pci/controller/pci-v3-semi.c
@@ -613,28 +613,30 @@ static int v3_pci_setup_resource(struct v3_pci *v3,
}
static int v3_get_dma_range_config(struct v3_pci *v3,
- struct of_pci_range *range,
+ struct resource_entry *entry,
u32 *pci_base, u32 *pci_map)
{
struct device *dev = v3->dev;
- u64 cpu_end = range->cpu_addr + range->size - 1;
- u64 pci_end = range->pci_addr + range->size - 1;
+ u64 cpu_addr = entry->res->start;
+ u64 cpu_end = entry->res->end;
+ u64 pci_end = cpu_end - entry->offset;
+ u64 pci_addr = entry->res->start - entry->offset;
u32 val;
- if (range->pci_addr & ~V3_PCI_BASE_M_ADR_BASE) {
+ if (pci_addr & ~V3_PCI_BASE_M_ADR_BASE) {
dev_err(dev, "illegal range, only PCI bits 31..20 allowed\n");
return -EINVAL;
}
- val = ((u32)range->pci_addr) & V3_PCI_BASE_M_ADR_BASE;
+ val = ((u32)pci_addr) & V3_PCI_BASE_M_ADR_BASE;
*pci_base = val;
- if (range->cpu_addr & ~V3_PCI_MAP_M_MAP_ADR) {
+ if (cpu_addr & ~V3_PCI_MAP_M_MAP_ADR) {
dev_err(dev, "illegal range, only CPU bits 31..20 allowed\n");
return -EINVAL;
}
- val = ((u32)range->cpu_addr) & V3_PCI_MAP_M_MAP_ADR;
+ val = ((u32)cpu_addr) & V3_PCI_MAP_M_MAP_ADR;
- switch (range->size) {
+ switch (resource_size(entry->res)) {
case SZ_1M:
val |= V3_LB_BASE_ADR_SIZE_1MB;
break;
@@ -682,8 +684,8 @@ static int v3_get_dma_range_config(struct v3_pci *v3,
dev_dbg(dev,
"DMA MEM CPU: 0x%016llx -> 0x%016llx => "
"PCI: 0x%016llx -> 0x%016llx base %08x map %08x\n",
- range->cpu_addr, cpu_end,
- range->pci_addr, pci_end,
+ cpu_addr, cpu_end,
+ pci_addr, pci_end,
*pci_base, *pci_map);
return 0;
@@ -692,24 +694,16 @@ static int v3_get_dma_range_config(struct v3_pci *v3,
static int v3_pci_parse_map_dma_ranges(struct v3_pci *v3,
struct device_node *np)
{
- struct of_pci_range range;
- struct of_pci_range_parser parser;
+ struct pci_host_bridge *bridge = pci_host_bridge_from_priv(v3);
struct device *dev = v3->dev;
+ struct resource_entry *entry;
int i = 0;
- if (of_pci_dma_range_parser_init(&parser, np)) {
- dev_err(dev, "missing dma-ranges property\n");
- return -EINVAL;
- }
-
- /*
- * Get the dma-ranges from the device tree
- */
- for_each_of_pci_range(&parser, &range) {
+ resource_list_for_each_entry(entry, &bridge->dma_ranges) {
int ret;
u32 pci_base, pci_map;
- ret = v3_get_dma_range_config(v3, &range, &pci_base, &pci_map);
+ ret = v3_get_dma_range_config(v3, entry, &pci_base, &pci_map);
if (ret)
return ret;
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 09/11] PCI: xgene: Use inbound resources for setup
From: Rob Herring @ 2019-09-24 21:46 UTC (permalink / raw)
To: linux-pci, Bjorn Helgaas, Lorenzo Pieralisi; +Cc: Toan Le, linux-arm-kernel
In-Reply-To: <20190924214630.12817-1-robh@kernel.org>
Now that the helpers provide the inbound resources in the host bridge
'dma_ranges' resource list, convert the Xgene host bridge to use the
resource list to setup the inbound addresses.
Cc: Toan Le <toan@os.amperecomputing.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/pci/controller/pci-xgene.c | 32 ++++++++++--------------------
1 file changed, 10 insertions(+), 22 deletions(-)
diff --git a/drivers/pci/controller/pci-xgene.c b/drivers/pci/controller/pci-xgene.c
index 11f27c42c06a..09c913b12ebc 100644
--- a/drivers/pci/controller/pci-xgene.c
+++ b/drivers/pci/controller/pci-xgene.c
@@ -485,27 +485,27 @@ static int xgene_pcie_select_ib_reg(u8 *ib_reg_mask, u64 size)
}
static void xgene_pcie_setup_ib_reg(struct xgene_pcie_port *port,
- struct of_pci_range *range, u8 *ib_reg_mask)
+ struct resource_entry *entry, u8 *ib_reg_mask)
{
void __iomem *cfg_base = port->cfg_base;
struct device *dev = port->dev;
void *bar_addr;
u32 pim_reg;
- u64 cpu_addr = range->cpu_addr;
- u64 pci_addr = range->pci_addr;
- u64 size = range->size;
+ u64 cpu_addr = entry->res->start;
+ u64 pci_addr = cpu_addr - entry->offset;
+ u64 size = resource_size(entry->res);
u64 mask = ~(size - 1) | EN_REG;
u32 flags = PCI_BASE_ADDRESS_MEM_TYPE_64;
u32 bar_low;
int region;
- region = xgene_pcie_select_ib_reg(ib_reg_mask, range->size);
+ region = xgene_pcie_select_ib_reg(ib_reg_mask, size);
if (region < 0) {
dev_warn(dev, "invalid pcie dma-range config\n");
return;
}
- if (range->flags & IORESOURCE_PREFETCH)
+ if (entry->res->flags & IORESOURCE_PREFETCH)
flags |= PCI_BASE_ADDRESS_MEM_PREFETCH;
bar_low = pcie_bar_low_val((u32)cpu_addr, flags);
@@ -536,25 +536,13 @@ static void xgene_pcie_setup_ib_reg(struct xgene_pcie_port *port,
static int xgene_pcie_parse_map_dma_ranges(struct xgene_pcie_port *port)
{
- struct device_node *np = port->node;
- struct of_pci_range range;
- struct of_pci_range_parser parser;
- struct device *dev = port->dev;
+ struct pci_host_bridge *bridge = pci_host_bridge_from_priv(port);
+ struct resource_entry *entry;
u8 ib_reg_mask = 0;
- if (of_pci_dma_range_parser_init(&parser, np)) {
- dev_err(dev, "missing dma-ranges property\n");
- return -EINVAL;
- }
-
- /* Get the dma-ranges from DT */
- for_each_of_pci_range(&parser, &range) {
- u64 end = range.cpu_addr + range.size - 1;
+ resource_list_for_each_entry(entry, &bridge->dma_ranges)
+ xgene_pcie_setup_ib_reg(port, entry, &ib_reg_mask);
- dev_dbg(dev, "0x%08x 0x%016llx..0x%016llx -> 0x%016llx\n",
- range.flags, range.cpu_addr, end, range.pci_addr);
- xgene_pcie_setup_ib_reg(port, &range, &ib_reg_mask);
- }
return 0;
}
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 10/11] PCI: iproc: Use inbound resources for setup
From: Rob Herring @ 2019-09-24 21:46 UTC (permalink / raw)
To: linux-pci, Bjorn Helgaas, Lorenzo Pieralisi
Cc: Ray Jui, bcm-kernel-feedback-list, Scott Branden,
linux-arm-kernel
In-Reply-To: <20190924214630.12817-1-robh@kernel.org>
Now that the helpers provide the inbound resources in the host bridge
'dma_ranges' resource list, convert Broadcom iProc host bridge to use
the resource list to setup the inbound addresses.
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: bcm-kernel-feedback-list@broadcom.com
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/pci/controller/pcie-iproc.c | 63 ++++-------------------------
1 file changed, 8 insertions(+), 55 deletions(-)
diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c
index 2d457bfdaf66..9ed181050308 100644
--- a/drivers/pci/controller/pcie-iproc.c
+++ b/drivers/pci/controller/pcie-iproc.c
@@ -1122,15 +1122,15 @@ static int iproc_pcie_ib_write(struct iproc_pcie *pcie, int region_idx,
}
static int iproc_pcie_setup_ib(struct iproc_pcie *pcie,
- struct of_pci_range *range,
+ struct resource_entry *entry,
enum iproc_pcie_ib_map_type type)
{
struct device *dev = pcie->dev;
struct iproc_pcie_ib *ib = &pcie->ib;
int ret;
unsigned int region_idx, size_idx;
- u64 axi_addr = range->cpu_addr, pci_addr = range->pci_addr;
- resource_size_t size = range->size;
+ u64 axi_addr = entry->res->start, pci_addr = entry->res->start - entry->offset;
+ resource_size_t size = resource_size(entry->res);
/* iterate through all IARR mapping regions */
for (region_idx = 0; region_idx < ib->nr_regions; region_idx++) {
@@ -1182,66 +1182,19 @@ static int iproc_pcie_setup_ib(struct iproc_pcie *pcie,
return ret;
}
-static int iproc_pcie_add_dma_range(struct device *dev,
- struct list_head *resources,
- struct of_pci_range *range)
-{
- struct resource *res;
- struct resource_entry *entry, *tmp;
- struct list_head *head = resources;
-
- res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
- if (!res)
- return -ENOMEM;
-
- resource_list_for_each_entry(tmp, resources) {
- if (tmp->res->start < range->cpu_addr)
- head = &tmp->node;
- }
-
- res->start = range->cpu_addr;
- res->end = res->start + range->size - 1;
-
- entry = resource_list_create_entry(res, 0);
- if (!entry)
- return -ENOMEM;
-
- entry->offset = res->start - range->cpu_addr;
- resource_list_add(entry, head);
-
- return 0;
-}
-
static int iproc_pcie_map_dma_ranges(struct iproc_pcie *pcie)
{
struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
- struct of_pci_range range;
- struct of_pci_range_parser parser;
- int ret;
- LIST_HEAD(resources);
+ struct resource_entry *entry;
+ int ret = 0;
- /* Get the dma-ranges from DT */
- ret = of_pci_dma_range_parser_init(&parser, pcie->dev->of_node);
- if (ret)
- return ret;
-
- for_each_of_pci_range(&parser, &range) {
- ret = iproc_pcie_add_dma_range(pcie->dev,
- &resources,
- &range);
- if (ret)
- goto out;
+ resource_list_for_each_entry(entry, &host->dma_ranges) {
/* Each range entry corresponds to an inbound mapping region */
- ret = iproc_pcie_setup_ib(pcie, &range, IPROC_PCIE_IB_MAP_MEM);
+ ret = iproc_pcie_setup_ib(pcie, entry, IPROC_PCIE_IB_MAP_MEM);
if (ret)
- goto out;
+ break;
}
- list_splice_init(&resources, &host->dma_ranges);
-
- return 0;
-out:
- pci_free_resource_list(&resources);
return ret;
}
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 11/11] PCI: rcar: Use inbound resources for setup
From: Rob Herring @ 2019-09-24 21:46 UTC (permalink / raw)
To: linux-pci, Bjorn Helgaas, Lorenzo Pieralisi
Cc: Simon Horman, linux-arm-kernel
In-Reply-To: <20190924214630.12817-1-robh@kernel.org>
Now that the helpers provide the inbound resources in the host bridge
'dma_ranges' resource list, convert Renesas R-Car PCIe host bridge to
use the resource list to setup the inbound addresses.
Cc: Simon Horman <horms@verge.net.au>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/pci/controller/pcie-rcar.c | 45 +++++++++++-------------------
1 file changed, 16 insertions(+), 29 deletions(-)
diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
index b8d6e86a5539..453c931aaf77 100644
--- a/drivers/pci/controller/pcie-rcar.c
+++ b/drivers/pci/controller/pcie-rcar.c
@@ -1014,16 +1014,16 @@ static int rcar_pcie_get_resources(struct rcar_pcie *pcie)
}
static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie,
- struct of_pci_range *range,
+ struct resource_entry *entry,
int *index)
{
- u64 restype = range->flags;
- u64 cpu_addr = range->cpu_addr;
- u64 cpu_end = range->cpu_addr + range->size;
- u64 pci_addr = range->pci_addr;
+ u64 restype = entry->res->flags;
+ u64 cpu_addr = entry->res->start;
+ u64 cpu_end = entry->res->end;
+ u64 pci_addr = entry->res->start - entry->offset;
u32 flags = LAM_64BIT | LAR_ENABLE;
u64 mask;
- u64 size;
+ u64 size = resource_size(entry->res);
int idx = *index;
if (restype & IORESOURCE_PREFETCH)
@@ -1037,9 +1037,7 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie,
unsigned long nr_zeros = __ffs64(cpu_addr);
u64 alignment = 1ULL << nr_zeros;
- size = min(range->size, alignment);
- } else {
- size = range->size;
+ size = min(size, alignment);
}
/* Hardware supports max 4GiB inbound region */
size = min(size, 1ULL << 32);
@@ -1078,30 +1076,19 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie,
return 0;
}
-static int rcar_pcie_parse_map_dma_ranges(struct rcar_pcie *pcie,
- struct device_node *np)
+static int rcar_pcie_parse_map_dma_ranges(struct rcar_pcie *pcie)
{
- struct of_pci_range range;
- struct of_pci_range_parser parser;
- int index = 0;
- int err;
-
- if (of_pci_dma_range_parser_init(&parser, np))
- return -EINVAL;
-
- /* Get the dma-ranges from DT */
- for_each_of_pci_range(&parser, &range) {
- u64 end = range.cpu_addr + range.size - 1;
-
- dev_dbg(pcie->dev, "0x%08x 0x%016llx..0x%016llx -> 0x%016llx\n",
- range.flags, range.cpu_addr, end, range.pci_addr);
+ struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
+ struct resource_entry *entry;
+ int index = 0, err = 0;
- err = rcar_pcie_inbound_ranges(pcie, &range, &index);
+ resource_list_for_each_entry(entry, &bridge->dma_ranges) {
+ err = rcar_pcie_inbound_ranges(pcie, entry, &index);
if (err)
- return err;
+ break;
}
- return 0;
+ return err;
}
static const struct of_device_id rcar_pcie_of_match[] = {
@@ -1162,7 +1149,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
goto err_unmap_msi_irqs;
}
- err = rcar_pcie_parse_map_dma_ranges(pcie, dev->of_node);
+ err = rcar_pcie_parse_map_dma_ranges(pcie);
if (err)
goto err_clk_disable;
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 00/11] of: Fix DMA configuration for non-DT masters
From: Rob Herring @ 2019-09-24 21:59 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: devicetree, Matthias Brugger, linux-arm-msm, freedreno,
linux-wireless, linux-kernel@vger.kernel.org, dri-devel, etnaviv,
linux-tegra, Florian Fainelli, Stefan Wahren, james.quinlan,
linux-pci, open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM,
xen-devel, Dan Williams, Robin Murphy, Frank Rowand,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Linux Media Mailing List
In-Reply-To: <20190924181244.7159-1-nsaenzjulienne@suse.de>
On Tue, Sep 24, 2019 at 1:12 PM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Hi All,
> this series tries to address one of the issues blocking us from
> upstreaming Broadcom's STB PCIe controller[1]. Namely, the fact that
> devices not represented in DT which sit behind a PCI bus fail to get the
> bus' DMA addressing constraints.
>
> This is due to the fact that of_dma_configure() assumes it's receiving a
> DT node representing the device being configured, as opposed to the PCIe
> bridge node we currently pass. This causes the code to directly jump
> into PCI's parent node when checking for 'dma-ranges' and misses
> whatever was set there.
>
> To address this I create a new API in OF - inspired from Robin Murphys
> original proposal[2] - which accepts a bus DT node as it's input in
> order to configure a device's DMA constraints. The changes go deep into
> of/address.c's implementation, as a device being having a DT node
> assumption was pretty strong.
>
> On top of this work, I also cleaned up of_dma_configure() removing its
> redundant arguments and creating an alternative function for the special cases
> not applicable to either the above case or the default usage.
>
> IMO the resulting functions are more explicit. They will probably
> surface some hacky usages that can be properly fixed as I show with the
> DT fixes on the Layerscape platform.
>
> This was also tested on a Raspberry Pi 4 with a custom PCIe driver and
> on a Seattle AMD board.
Humm, I've been working on this issue too. Looks similar though yours
has a lot more churn and there's some other bugs I've found.
Can you test out this branch[1]. I don't have any h/w needing this,
but wrote a unittest and tested with modified QEMU.
Rob
[1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git dma-masks
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 1/1] i2c: iproc: Add i2c repeated start capability
From: Ray Jui @ 2019-09-24 22:23 UTC (permalink / raw)
To: Wolfram Sang
Cc: Mark Rutland, devicetree, Lori Hikichi, Florian Fainelli,
Shivaraj Shetty, Rayagonda Kokatanur, linux-kernel, Icarus Chau,
Rob Herring, bcm-kernel-feedback-list, linux-i2c,
linux-arm-kernel
In-Reply-To: <20190924185757.GA1538@kunai>
On 9/24/19 11:57 AM, Wolfram Sang wrote:
>
>> In my opinion, it's probably better to continue to support master_xfer in
>> our driver (with obvious limitations), in order to allow i2ctransfer (or any
>> apps that use I2C RDWR) to continue to work.
>>
>> What do you think?
>
> Yes, don't break it for users. We should have paid more attention to it
> in the beginning. But, while not ideal, it is not such a big deal to
> keep it like this.
>
> Thanks for your investigations!
>
Thanks, Wolfram.
Let's please continue with the review process on the current patch then?
Note the patch was already internally reviewed by me.
Please help to review it and let us know if there's any change that
needs to be made?
Regards,
Ray
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: imx6qdl-udoo: Add Pincfgs for OTG
From: Shawn Guo @ 2019-09-24 23:09 UTC (permalink / raw)
To: Markus Kueffner
Cc: Mark Rutland, devicetree, Sascha Hauer, linux-kernel, Rob Herring,
NXP Linux Team, Pengutronix Kernel Team, Fabio Estevam,
linux-arm-kernel
In-Reply-To: <20190916130604.GA3140@ubuntu-buildhost>
On Mon, Sep 16, 2019 at 01:06:04PM +0000, Markus Kueffner wrote:
> On Mon, Apr 15, 2019 at 05:11:51PM +0800, Shawn Guo wrote:
> > On Sat, Apr 13, 2019 at 03:19:36PM +0200, Markus Kueffner wrote:
> > > Add Pincfgs to enable the i.MX6's OTG feature for UDOO
> > >
> > > Signed-off-by: Markus Kueffner <kueffner.markus@gmail.com>
> >
> > Applied, thanks.
>
> Hello,
>
> I was wondering when this might get merged into a release.
> Is there anything else I need to fix?
I'm sorry, Markus. There was something wrong on my side, and the patch
did not really get applied for 5.4. I just fixed it for the next
release. Sorry about that!
Shawn
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] ARM: OMAP2+: Fix warnings with broken omap2_set_init_voltage()
From: Tony Lindgren @ 2019-09-24 23:32 UTC (permalink / raw)
To: linux-omap
Cc: Nishanth Menon, H. Nikolaus Schaller, Tero Kristo,
André Roth, Adam Ford, linux-arm-kernel
This code is currently unable to find the dts opp tables as ti-cpufreq
needs to set them up first based on speed binning.
We stopped initializing the opp tables with platform code years ago for
device tree based booting with commit 92d51856d740 ("ARM: OMAP3+: do not
register non-dt OPP tables for device tree boot"), and all of mach-omap2
is now booting using device tree.
We currently get the following errors on init:
omap2_set_init_voltage: unable to find boot up OPP for vdd_mpu
omap2_set_init_voltage: unable to set vdd_mpu
omap2_set_init_voltage: unable to find boot up OPP for vdd_core
omap2_set_init_voltage: unable to set vdd_core
omap2_set_init_voltage: unable to find boot up OPP for vdd_iva
omap2_set_init_voltage: unable to set vdd_iva
Let's just drop the unused code. Nowadays ti-cpufreq should be used to
to initialize things properly.
Cc: Adam Ford <aford173@gmail.com>
Cc: André Roth <neolynx@gmail.com>
Cc: "H. Nikolaus Schaller" <hns@goldelico.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Guys, please check and ack if we can really do this to get rid of some
pointless dmesg -l3 errors without affecting the ongoing cpufreq and
voltage work.
---
arch/arm/mach-omap2/pm.c | 100 ---------------------------------------
1 file changed, 100 deletions(-)
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -74,83 +74,6 @@ int omap_pm_clkdms_setup(struct clockdomain *clkdm, void *unused)
return 0;
}
-/*
- * This API is to be called during init to set the various voltage
- * domains to the voltage as per the opp table. Typically we boot up
- * at the nominal voltage. So this function finds out the rate of
- * the clock associated with the voltage domain, finds out the correct
- * opp entry and sets the voltage domain to the voltage specified
- * in the opp entry
- */
-static int __init omap2_set_init_voltage(char *vdd_name, char *clk_name,
- const char *oh_name)
-{
- struct voltagedomain *voltdm;
- struct clk *clk;
- struct dev_pm_opp *opp;
- unsigned long freq, bootup_volt;
- struct device *dev;
-
- if (!vdd_name || !clk_name || !oh_name) {
- pr_err("%s: invalid parameters\n", __func__);
- goto exit;
- }
-
- if (!strncmp(oh_name, "mpu", 3))
- /*
- * All current OMAPs share voltage rail and clock
- * source, so CPU0 is used to represent the MPU-SS.
- */
- dev = get_cpu_device(0);
- else
- dev = omap_device_get_by_hwmod_name(oh_name);
-
- if (IS_ERR(dev)) {
- pr_err("%s: Unable to get dev pointer for hwmod %s\n",
- __func__, oh_name);
- goto exit;
- }
-
- voltdm = voltdm_lookup(vdd_name);
- if (!voltdm) {
- pr_err("%s: unable to get vdd pointer for vdd_%s\n",
- __func__, vdd_name);
- goto exit;
- }
-
- clk = clk_get(NULL, clk_name);
- if (IS_ERR(clk)) {
- pr_err("%s: unable to get clk %s\n", __func__, clk_name);
- goto exit;
- }
-
- freq = clk_get_rate(clk);
- clk_put(clk);
-
- opp = dev_pm_opp_find_freq_ceil(dev, &freq);
- if (IS_ERR(opp)) {
- pr_err("%s: unable to find boot up OPP for vdd_%s\n",
- __func__, vdd_name);
- goto exit;
- }
-
- bootup_volt = dev_pm_opp_get_voltage(opp);
- dev_pm_opp_put(opp);
-
- if (!bootup_volt) {
- pr_err("%s: unable to find voltage corresponding to the bootup OPP for vdd_%s\n",
- __func__, vdd_name);
- goto exit;
- }
-
- voltdm_scale(voltdm, bootup_volt);
- return 0;
-
-exit:
- pr_err("%s: unable to set vdd_%s\n", __func__, vdd_name);
- return -EINVAL;
-}
-
#ifdef CONFIG_SUSPEND
static int omap_pm_enter(suspend_state_t suspend_state)
{
@@ -208,25 +131,6 @@ void omap_common_suspend_init(void *pm_suspend)
}
#endif /* CONFIG_SUSPEND */
-static void __init omap3_init_voltages(void)
-{
- if (!soc_is_omap34xx())
- return;
-
- omap2_set_init_voltage("mpu_iva", "dpll1_ck", "mpu");
- omap2_set_init_voltage("core", "l3_ick", "l3_main");
-}
-
-static void __init omap4_init_voltages(void)
-{
- if (!soc_is_omap44xx())
- return;
-
- omap2_set_init_voltage("mpu", "dpll_mpu_ck", "mpu");
- omap2_set_init_voltage("core", "l3_div_ck", "l3_main_1");
- omap2_set_init_voltage("iva", "dpll_iva_m5x2_ck", "iva");
-}
-
int __maybe_unused omap_pm_nop_init(void)
{
return 0;
@@ -246,10 +150,6 @@ int __init omap2_common_pm_late_init(void)
omap4_twl_init();
omap_voltage_late_init();
- /* Initialize the voltages */
- omap3_init_voltages();
- omap4_init_voltages();
-
/* Smartreflex device init */
omap_devinit_smartreflex();
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH V3 09/10] interconnect: mediatek: Add mt8183 interconnect provider driver
From: Georgi Djakov @ 2019-09-24 23:53 UTC (permalink / raw)
To: Henry Chen, Rob Herring, Matthias Brugger, Viresh Kumar,
Stephen Boyd, Ryan Case
Cc: James Liao, Weiyi Lu, Nicolas Boichat, Linux PM list,
linux-kernel, Fan Chen, devicetree, linux-mediatek,
linux-arm-kernel
In-Reply-To: <1566995328-15158-10-git-send-email-henryc.chen@mediatek.com>
Hi Henry,
Please CC also the linux-pm@ list.
On 8/28/19 05:28, Henry Chen wrote:
> Introduce Mediatek MT8183 specific provider driver using the
> interconnect framework.
>
> Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
> ---
> drivers/interconnect/Kconfig | 1 +
> drivers/interconnect/Makefile | 1 +
> drivers/interconnect/mediatek/Kconfig | 13 ++
> drivers/interconnect/mediatek/Makefile | 3 +
> drivers/interconnect/mediatek/mtk-emi.c | 246 ++++++++++++++++++++++++++++++++
> 5 files changed, 264 insertions(+)
> create mode 100644 drivers/interconnect/mediatek/Kconfig
> create mode 100644 drivers/interconnect/mediatek/Makefile
> create mode 100644 drivers/interconnect/mediatek/mtk-emi.c
>
[..]
> +
> +#define MT8183_MAX_LINKS 6
Looks like 1 is enough. Sorry for missing this in my earlier review.
> +
> +/**
> + * struct mtk_icc_node - Mediatek specific interconnect nodes
> + * @name: the node name used in debugfs
> + * @ep: true if the node is an end point.
> + * @id: a unique node identifier
> + * @links: an array of nodes where we can go next while traversing
> + * @num_links: the total number of @links
> + * @buswidth: width of the interconnect between a node and the bus
> + * @sum_avg: current sum aggregate value of all avg bw kBps requests
> + * @max_peak: current max aggregate value of all peak bw kBps requests
> + */
> +struct mtk_icc_node {
> + unsigned char *name;
> + bool ep;
> + u16 id;
> + u16 links[MT8183_MAX_LINKS];
> + u16 num_links;
> + u16 buswidth;
> + u64 sum_avg;
> + u64 max_peak;
> +};
> +
> +struct mtk_icc_desc {
> + struct mtk_icc_node **nodes;
> + size_t num_nodes;
> +};
> +
> +#define DEFINE_MNODE(_name, _id, _buswidth, _ep, ...) \
> + static struct mtk_icc_node _name = { \
> + .name = #_name, \
> + .id = _id, \
> + .buswidth = _buswidth, \
> + .ep = _ep, \
> + .num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })), \
> +}
> +
> +DEFINE_MNODE(ddr_emi, SLAVE_DDR_EMI, 1024, 1, 0);
> +DEFINE_MNODE(mcusys, MASTER_MCUSYS, 256, 0, SLAVE_DDR_EMI);
> +DEFINE_MNODE(gpu, MASTER_GPUSYS, 256, 0, SLAVE_DDR_EMI);
> +DEFINE_MNODE(mmsys, MASTER_MMSYS, 256, 0, SLAVE_DDR_EMI);
> +DEFINE_MNODE(mm_vpu, MASTER_MM_VPU, 128, 0, MASTER_MMSYS);
> +DEFINE_MNODE(mm_disp, MASTER_MM_DISP, 128, 0, MASTER_MMSYS);
> +DEFINE_MNODE(mm_vdec, MASTER_MM_VDEC, 128, 0, MASTER_MMSYS);
> +DEFINE_MNODE(mm_venc, MASTER_MM_VENC, 128, 0, MASTER_MMSYS);
> +DEFINE_MNODE(mm_cam, MASTER_MM_CAM, 128, 0, MASTER_MMSYS);
> +DEFINE_MNODE(mm_img, MASTER_MM_IMG, 128, 0, MASTER_MMSYS);
> +DEFINE_MNODE(mm_mdp, MASTER_MM_MDP, 128, 0, MASTER_MMSYS);
> +
[..]
> +static int emi_icc_aggregate(struct icc_node *node, u32 avg_bw,
> + u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
> +{
The prototype of this function has changed meanwhile, so you might want to update.
[..]
> +static int emi_icc_probe(struct platform_device *pdev)
> +{
> + int ret;
> + const struct mtk_icc_desc *desc;
> + struct icc_node *node;
> + struct icc_onecell_data *data;
> + struct icc_provider *provider;
> + struct mtk_icc_node **mnodes;
> + size_t num_nodes, i, j;
> +
> + desc = of_device_get_match_data(&pdev->dev);
> + if (!desc)
> + return -EINVAL;
> +
> + mnodes = desc->nodes;
> + num_nodes = desc->num_nodes;
> +
> + provider = devm_kzalloc(&pdev->dev, sizeof(*provider), GFP_KERNEL);
> + if (!provider)
> + return -ENOMEM;
> +
> + data = devm_kcalloc(&pdev->dev, num_nodes, sizeof(*node), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + provider->dev = &pdev->dev;
> + provider->set = emi_icc_set;
> + provider->aggregate = emi_icc_aggregate;
> + provider->xlate = of_icc_xlate_onecell;
> + INIT_LIST_HEAD(&provider->nodes);
> + provider->data = data;
> +
> + ret = icc_provider_add(provider);
> + if (ret) {
> + dev_err(&pdev->dev, "error adding interconnect provider\n");
> + return ret;
> + }
> +
> + for (i = 0; i < num_nodes; i++) {
> + node = icc_node_create(mnodes[i]->id);
> + if (IS_ERR(node)) {
> + ret = PTR_ERR(node);
> + goto err;
> + }
> +
> + node->name = mnodes[i]->name;
> + node->data = mnodes[i];
> + icc_node_add(node, provider);
> +
> + dev_dbg(&pdev->dev, "registered node %s, num link: %d\n",
> + mnodes[i]->name, mnodes[i]->num_links);
> +
> + /* populate links */
> + for (j = 0; j < mnodes[i]->num_links; j++)
> + icc_link_create(node, mnodes[i]->links[j]);
> +
> + data->nodes[i] = node;
> + }
> + data->num_nodes = num_nodes;
> +
> + platform_set_drvdata(pdev, provider);
> +
> + return 0;
> +err:
> + list_for_each_entry(node, &provider->nodes, node_list) {
> + icc_node_del(node);
> + icc_node_destroy(node->id);
> + }
> +
> + icc_provider_del(provider);
You can just call emi_icc_remove() instead of the above.
Thanks,
Georgi
> + return ret;
> +}
> +
> +static int emi_icc_remove(struct platform_device *pdev)
> +{
> + struct icc_provider *provider = platform_get_drvdata(pdev);
> + struct icc_node *n;
> +
> + list_for_each_entry(n, &provider->nodes, node_list) {
> + icc_node_del(n);
> + icc_node_destroy(n->id);
> + }
> +
> + return icc_provider_del(provider);
> +}
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] PM / devfreq: Lock devfreq in trans_stat_show
From: Chanwoo Choi @ 2019-09-25 1:21 UTC (permalink / raw)
To: Leonard Crestez, Matthias Kaehlcke
Cc: Artur Świgoń, linux-pm@vger.kernel.org,
Krzysztof Kozlowski, Lukasz Luba, Kyungmin Park, MyungJoo Ham,
dl-linux-imx, Georgi Djakov, linux-arm-kernel@lists.infradead.org
In-Reply-To: <VI1PR04MB7023BF1AD2C61C8A5ABAD5FEEE840@VI1PR04MB7023.eurprd04.prod.outlook.com>
On 19. 9. 24. 오후 4:44, Leonard Crestez wrote:
> On 2019-09-24 5:07 AM, Chanwoo Choi wrote:
>> Hi,
>>
>> On 19. 9. 24. 오전 1:27, Leonard Crestez wrote:
>>> There is no locking in this sysfs show function so stats printing can
>>> race with a devfreq_update_status called as part of freq switching or
>>> with initialization.
>>>
>>> Also add an assert in devfreq_update_status to make it clear that lock
>>> must be held by caller.
>>>
>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>> ---
>>> drivers/devfreq/devfreq.c | 13 ++++++++++---
>>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>>
>>> Changes since v1:
>>> * Split from series: low-priority bugfix not strictly required for PM QoS
>>> * Only keep lock during update, release before sprintf
>>>
>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>> index 4c58fbf7d4e4..00fc23fea5b2 100644
>>> --- a/drivers/devfreq/devfreq.c
>>> +++ b/drivers/devfreq/devfreq.c
>>> @@ -206,10 +206,11 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq)
>>> {
>>> int lev, prev_lev, ret = 0;
>>> unsigned long cur_time;
>>>
>>> cur_time = jiffies;
>>> + lockdep_assert_held(&devfreq->lock);
>>
>> It better to move lock checking before 'cur_time = jiffies'
>> in order to reduce the redundant code execution.
>
> OK but I don't see how this makes a difference for an assert? It just
> prints a warning and carries on.
According to the sequence between 'lockdep_assert_held' and 'cur_time = jiffies',
cur_time will be initialized with different jiffies because 'jiffies' is continuously
changed. In order to get the more correct time from 'jiffies',
we better to get 'jiffies' after releasing the lock.
>
>>> /* Immediately exit if previous_freq is not initialized yet. */
>>> if (!devfreq->previous_freq)
>>> goto out;
>>>
>>> @@ -1507,16 +1508,22 @@ static ssize_t trans_stat_show(struct device *dev,
>>> struct devfreq *devfreq = to_devfreq(dev);
>>> ssize_t len;
>>> int i, j;
>>> unsigned int max_state = devfreq->profile->max_state;
>>>
>>> - if (!devfreq->stop_polling &&
>>> - devfreq_update_status(devfreq, devfreq->previous_freq))
>>> - return 0;
>>> if (max_state == 0)
>>> return sprintf(buf, "Not Supported.\n");
>>>
>>> + /* lock and update */
>>
>> It is not necessary. Anyone can know that this code is related to mutex lock/unlock.
>
> OK. You're the second person to mention this but it's quite strange to
> see objections raised against comments.
The comment is very important to understand the code
for everyone. But, in this case, almost people understand
the usage of mutex_lock/mutex_unlock. It is no difficult
to understand the meaning of below codes.
Usually, we would add the comments if some codes are very difficult
without comments or some codes have depend on some call sequence and so on.
>
>>> + mutex_lock(&devfreq->lock);
>>> + if (!devfreq->stop_polling &&
>>> + devfreq_update_status(devfreq, devfreq->previous_freq)) {
>>> + mutex_unlock(&devfreq->lock);
>>> + return 0;
>>> + }
>>> + mutex_unlock(&devfreq->lock);
>>> +
>>> len = sprintf(buf, " From : To\n");
>>> len += sprintf(buf + len, " :");
>>> for (i = 0; i < max_state; i++)
>>> len += sprintf(buf + len, "%10lu",
>>> devfreq->profile->freq_table[i]);
>>>
>>
>> Basically, it is necessary. Please edit it by my comments.
>> Also, you have to add the following 'fixes' as following:
>> and send it stable mailing list.
>>
>> Fixes: 39688ce6facd ("PM / devfreq: account suspend/resume for stats")
>>
>> If you edit it by my comments, feel free to add my tag:
>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 4/6] PM / devfreq: Introduce get_freq_range helper
From: Chanwoo Choi @ 2019-09-25 1:37 UTC (permalink / raw)
To: Leonard Crestez, MyungJoo Ham, Kyungmin Park, Matthias Kaehlcke
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, NXP Linux Team, Krzysztof Kozlowski, Lukasz Luba,
Alexandre Bailon, Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <674fe91065034083fd7c8c1810305cd01551bb80.1569319738.git.leonard.crestez@nxp.com>
On 19. 9. 24. 오후 7:11, Leonard Crestez wrote:
> Moving handling of min/max freq to a single function and call it from
> update_devfreq and for printing min/max freq values in sysfs.
>
> This changes the behavior of out-of-range min_freq/max_freq: clamping
> is now done at evaluation time. This means that if an out-of-range
> constraint is imposed by sysfs and it later becomes valid then it will
> be enforced.
>
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> drivers/devfreq/devfreq.c | 112 ++++++++++++++++++++++----------------
> 1 file changed, 64 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 4a878baa809e..eee403e70c84 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -96,10 +96,54 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq)
> dev_pm_opp_put(opp);
>
> return max_freq;
> }
>
> +/**
> + * get_freq_range() - Get the current freq range
> + * @devfreq: the devfreq instance
> + * @min_freq: the min frequency
> + * @max_freq: the max frequency
> + *
> + * This takes into consideration all constraints.
> + */
> +static void get_freq_range(struct devfreq *devfreq,
> + unsigned long *min_freq,
> + unsigned long *max_freq)
> +{
> + unsigned long *freq_table = devfreq->profile->freq_table;
> +
> + lockdep_assert_held(&devfreq->lock);
> +
> + /*
> + * Init min/max frequency from freq table.
Init -> Initialize
min/max -> minimum/maximum
> + * Drivers can initialize this in either ascending or descending order
Drivers -> devfreq drivers
> + * and devfreq core supports both.
> + */
In result, I prefer to change the comments as following:
/*
* Initialize the minimum/maximum frequency from freq_table.
* The devfreq drivers can initialize freq_table in either
* ascending or descending order and devfreq core supports both.
*/
> + if (freq_table[0] < freq_table[devfreq->profile->max_state - 1]) {
> + *min_freq = freq_table[0];
> + *max_freq = freq_table[devfreq->profile->max_state - 1];
> + } else {
> + *min_freq = freq_table[devfreq->profile->max_state - 1];
> + *max_freq = freq_table[0];
> + }
> +
> + /* constraints from sysfs */
'constraints' -> Constraint because first verb have to be used
as the sigular verbs. Also, I think that have to enhance the comments
I prefer to use following comments:
/* Constraint minimum/maximum frequency from user input via sysfs */
> + *min_freq = max(*min_freq, devfreq->min_freq);
> + *max_freq = min(*max_freq, devfreq->max_freq);
> +
> + /* constraints from OPP interface */
ditto. I prefer to use following comments:
/* Constraint minimum/maximum frequency from OPP interface */
> + *min_freq = max(*min_freq, devfreq->scaling_min_freq);
> + /* scaling_max_freq can be zero on error */
Please remove it.
> + if (devfreq->scaling_max_freq)
As I knew, devfreq->scaling_max_freq is never zero.
So, it is always true. This if statement is needed.
> + *max_freq = min(*max_freq, devfreq->scaling_max_freq);
> +
> + /* max_freq takes precedence over min_freq */
As I said, almost people know that min_freq have be under than max_freq.
Please remove it. And until finishing the discussion on mailing list,
please don't send the next version. If you just replied from my comment
and then wait my next comment, we can save the time without replying
the repetitive and same comment for same point.
> + if (*min_freq > *max_freq)
> + *min_freq = *max_freq;
> +}
> +
> /**
> * devfreq_get_freq_level() - Lookup freq_table for the frequency
> * @devfreq: the devfreq instance
> * @freq: the target frequency
> */
> @@ -349,20 +393,11 @@ int update_devfreq(struct devfreq *devfreq)
>
> /* Reevaluate the proper frequency */
> err = devfreq->governor->get_target_freq(devfreq, &freq);
> if (err)
> return err;
> -
> - /*
> - * Adjust the frequency with user freq, QoS and available freq.
> - *
> - * List from the highest priority
> - * max_freq
> - * min_freq
> - */
> - max_freq = min(devfreq->scaling_max_freq, devfreq->max_freq);
> - min_freq = max(devfreq->scaling_min_freq, devfreq->min_freq);
> + get_freq_range(devfreq, &min_freq, &max_freq);
>
> if (freq < min_freq) {
> freq = min_freq;
> flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
> }
> @@ -1298,40 +1333,28 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
> ret = sscanf(buf, "%lu", &value);
> if (ret != 1)
> return -EINVAL;
>
> mutex_lock(&df->lock);
> -
> - if (value) {
> - if (value > df->max_freq) {
> - ret = -EINVAL;
> - goto unlock;
> - }
> - } else {
> - unsigned long *freq_table = df->profile->freq_table;
> -
> - /* Get minimum frequency according to sorting order */
> - if (freq_table[0] < freq_table[df->profile->max_state - 1])
> - value = freq_table[0];
> - else
> - value = freq_table[df->profile->max_state - 1];
> - }
> -
> df->min_freq = value;
> update_devfreq(df);
> - ret = count;
> -unlock:
> mutex_unlock(&df->lock);
> - return ret;
> +
> + return count;
> }
>
> static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> struct devfreq *df = to_devfreq(dev);
> + unsigned long min_freq, max_freq;
> +
> + mutex_lock(&df->lock);
> + get_freq_range(df, &min_freq, &max_freq);
> + mutex_unlock(&df->lock);
>
> - return sprintf(buf, "%lu\n", max(df->scaling_min_freq, df->min_freq));
> + return sprintf(buf, "%lu\n", min_freq);
> }
>
> static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> @@ -1343,40 +1366,33 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
> if (ret != 1)
> return -EINVAL;
>
> mutex_lock(&df->lock);
>
> - if (value) {
> - if (value < df->min_freq) {
> - ret = -EINVAL;
> - goto unlock;
> - }
> - } else {
> - unsigned long *freq_table = df->profile->freq_table;
> -
> - /* Get maximum frequency according to sorting order */
> - if (freq_table[0] < freq_table[df->profile->max_state - 1])
> - value = freq_table[df->profile->max_state - 1];
> - else
> - value = freq_table[0];
> - }
> + /* Interpret zero as "don't care" */
Please remove it. Also, the detailed comment for user have to add
the documentation file.
> + if (!value)
> + value = ULONG_MAX;
>
> df->max_freq = value;
> update_devfreq(df);
> - ret = count;
> -unlock:
> mutex_unlock(&df->lock);
> - return ret;
> +
> + return count;
> }
> static DEVICE_ATTR_RW(min_freq);
>
> static ssize_t max_freq_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> struct devfreq *df = to_devfreq(dev);
> + unsigned long min_freq, max_freq;
> +
> + mutex_lock(&df->lock);
> + get_freq_range(df, &min_freq, &max_freq);
> + mutex_unlock(&df->lock);
>
> - return sprintf(buf, "%lu\n", min(df->scaling_max_freq, df->max_freq));
> + return sprintf(buf, "%lu\n", max_freq);
> }
> static DEVICE_ATTR_RW(max_freq);
>
> static ssize_t available_frequencies_show(struct device *d,
> struct device_attribute *attr,
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 0/6] PM / devfreq: Add dev_pm_qos support
From: Chanwoo Choi @ 2019-09-25 1:44 UTC (permalink / raw)
To: Leonard Crestez, MyungJoo Ham, Kyungmin Park, Matthias Kaehlcke
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, NXP Linux Team, Krzysztof Kozlowski, Lukasz Luba,
Alexandre Bailon, Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <cover.1569319738.git.leonard.crestez@nxp.com>
Hi Leonard,
Basically, I think that these series are very important.
But, you better to send the next version patch
after finishing the review/discussion on previous version.
I reviewed the v7 and then you replied your comment.
It is OK. But, you just send v8 without waiting my comment
from your reply. It is not efficient discussion method.
If we finish the review of some point in the v7,
it doesn't need to discuss the same comment on v8.
Please wait the reply for review. I think that
it can save the our time for the review and contribution.
On 19. 9. 24. 오후 7:11, Leonard Crestez wrote:
> Add dev_pm_qos notifiers to devfreq core in order to support frequency
> limits via dev_pm_qos_add_request.
>
> Unlike the rest of devfreq the dev_pm_qos frequency is measured in Khz,
> this is consistent with current dev_pm_qos usage for cpufreq and
> allows frequencies above 2Ghz (pm_qos expresses limits as s32).
>
> Like with cpufreq the handling of min_freq/max_freq is moved to the
> dev_pm_qos mechanism. Constraints from userspace are no longer clamped on
> store, instead all values can be written and we only check against OPPs in a
> new devfreq_get_freq_range function. This is consistent with the design of
> dev_pm_qos.
>
> Notifiers from pm_qos are executed under a single global dev_pm_qos_mtx and
> need to take devfreq->lock. Notifier registration takes the same dev_pm_qos_mtx
> so in order to prevent lockdep warnings it must be done outside devfreq->lock.
> Current devfreq_add_device does all initialization under devfreq->lock and that
> needs to be relaxed.
>
> ---
> Changes since v7:
> * Only #define HZ_PER_KHZ in patch where it's used.
> * Drop devfreq_ prefix for some internal functions.
> * Improve qos update error message.
> * Remove some unnecessary comments.
> * Collect reviews
> Link to v7: https://patchwork.kernel.org/cover/11157649/
>
> Changes since v6:
> * Don't return errno from devfreq_qos_notifier_call, return NOTIFY_DONE
> and print the error.
> * More spelling and punctuation nits
> Link to v6: https://patchwork.kernel.org/cover/11157201/
>
> Changes since v5:
> * Drop patches which are not strictly related to PM QoS.
> * Add a comment explaining why devfreq_add_device needs two cleanup paths.
> * Remove {} for single line.
> * Rename {min,max}_freq_req to user_{min,max}_freq_req
> * Collect reviews
> Link to v5: https://patchwork.kernel.org/cover/11149497/
>
> Sorry for forgetting to properly label v5. I know this is inside the
> merge window but review would still be appreciated.
>
> Changes since v4:
> * Move more devfreq_add_device init ahead of device_register.
> * Make devfreq_dev_release cleanup devices not yet in devfreq_list. This is
> simpler than previous attempt to add to devfreq_list sonner.
> * Take devfreq->lock in trans_stat_show
> * Register dev_pm_opp notifier on devfreq parent dev (which has OPPs)
> Link to v4: https://patchwork.kernel.org/cover/11114657/
>
> Changes since v4:
> * Move more devfreq_add_device init ahead of device_register.
> * Make devfreq_dev_release cleanup devices not yet in devfreq_list. This is
> simpler than previous attempt to add to devfreq_list sonner.
> * Take devfreq->lock in trans_stat_show
> * Register dev_pm_opp notifier on devfreq parent dev (which has OPPs)
> Like to v4: https://patchwork.kernel.org/cover/11114657/
>
> Changes since v3:
> * Cleanup locking and error-handling in devfreq_add_device
> * Register notifiers after device registration but before governor start
> * Keep the initialization of min_req/max_req ahead of device_register
> because it's used for sysfs handling
> * Use HZ_PER_KHZ instead of 1000
> * Add kernel-doc comments
> * Move OPP notifier to core
> Link to v3: https://patchwork.kernel.org/cover/11104061/
>
> Changes since v2:
> * Handle sysfs via dev_pm_qos (in separate patch)
> * Add locking to {min,max}_freq_show
> * Fix checkpatch issues (long lines etc)
> Link to v2: https://patchwork.kernel.org/patch/11084279/
>
> Changes since v1:
> * Add doxygen comments for min_nb/max_nb
> * Remove notifiers on error/cleanup paths. Keep gotos simple by relying on
> dev_pm_qos_remove_notifier ignoring notifiers which were not added.
> Link to v1: https://patchwork.kernel.org/patch/11078475/
>
> Leonard Crestez (6):
> PM / devfreq: Don't fail devfreq_dev_release if not in list
> PM / devfreq: Move more initialization before registration
> PM / devfreq: Don't take lock in devfreq_add_device
> PM / devfreq: Introduce get_freq_range helper
> PM / devfreq: Add PM QoS support
> PM / devfreq: Use PM QoS for sysfs min/max_freq
>
> drivers/devfreq/devfreq.c | 268 +++++++++++++++++++++++++-------------
> include/linux/devfreq.h | 14 +-
> 2 files changed, 191 insertions(+), 91 deletions(-)
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 2/6] PM / devfreq: Move more initialization before registration
From: Chanwoo Choi @ 2019-09-25 1:46 UTC (permalink / raw)
To: Leonard Crestez, MyungJoo Ham, Kyungmin Park, Matthias Kaehlcke
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, NXP Linux Team, Krzysztof Kozlowski, Lukasz Luba,
Alexandre Bailon, Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <398895f3b4f9ca4b03b47b56dfa25fbd58fd2311.1569319738.git.leonard.crestez@nxp.com>
Hi Leonard,
I copied and pasted my comments as following:
because you send new v8 version without waiting any reply.
In my case, I don't prefer to remove 'devm_' feature.
v7's comment.
I'm not sure that it is necessary. Because, mainline kernel
provides the resource-managed feature with 'devm_*'
in order to reduce the memory leak by user.
It's like going back to a time when 'devm_*' feature wasn't there.
I think just it depends on the coding style of author.
I leave the final decision of this patch to Myungjoo.
On 19. 9. 24. 오후 7:11, Leonard Crestez wrote:
> In general it is a better to initialize an object before making it
> accessible externally (through device_register).
>
> This makes it possible to avoid relying on locking a partially
> initialized object.
>
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> drivers/devfreq/devfreq.c | 43 +++++++++++++++++++++++----------------
> 1 file changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 12c4bcdc1f17..dbc6dc882aba 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -588,10 +588,12 @@ static void devfreq_dev_release(struct device *dev)
> mutex_unlock(&devfreq_list_lock);
>
> if (devfreq->profile->exit)
> devfreq->profile->exit(devfreq->dev.parent);
>
> + kfree(devfreq->time_in_state);
> + kfree(devfreq->trans_table);
> mutex_destroy(&devfreq->lock);
> kfree(devfreq);
> }
>
> /**
> @@ -671,44 +673,43 @@ struct devfreq *devfreq_add_device(struct device *dev,
> devfreq->max_freq = devfreq->scaling_max_freq;
>
> devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
> atomic_set(&devfreq->suspend_count, 0);
>
> - dev_set_name(&devfreq->dev, "devfreq%d",
> - atomic_inc_return(&devfreq_no));
> - err = device_register(&devfreq->dev);
> - if (err) {
> - mutex_unlock(&devfreq->lock);
> - put_device(&devfreq->dev);
> - goto err_out;
> - }
> -
> - devfreq->trans_table = devm_kzalloc(&devfreq->dev,
> + devfreq->trans_table = kzalloc(
> array3_size(sizeof(unsigned int),
> devfreq->profile->max_state,
> devfreq->profile->max_state),
> GFP_KERNEL);
> if (!devfreq->trans_table) {
> mutex_unlock(&devfreq->lock);
> err = -ENOMEM;
> - goto err_devfreq;
> + goto err_dev;
> }
>
> - devfreq->time_in_state = devm_kcalloc(&devfreq->dev,
> - devfreq->profile->max_state,
> - sizeof(unsigned long),
> - GFP_KERNEL);
> + devfreq->time_in_state = kcalloc(devfreq->profile->max_state,
> + sizeof(unsigned long),
> + GFP_KERNEL);
> if (!devfreq->time_in_state) {
> mutex_unlock(&devfreq->lock);
> err = -ENOMEM;
> - goto err_devfreq;
> + goto err_dev;
> }
>
> devfreq->last_stat_updated = jiffies;
>
> srcu_init_notifier_head(&devfreq->transition_notifier_list);
>
> + dev_set_name(&devfreq->dev, "devfreq%d",
> + atomic_inc_return(&devfreq_no));
> + err = device_register(&devfreq->dev);
> + if (err) {
> + mutex_unlock(&devfreq->lock);
> + put_device(&devfreq->dev);
> + goto err_out;
> + }
> +
> mutex_unlock(&devfreq->lock);
>
> mutex_lock(&devfreq_list_lock);
>
> governor = try_then_request_governor(devfreq->governor_name);
> @@ -734,14 +735,20 @@ struct devfreq *devfreq_add_device(struct device *dev,
>
> return devfreq;
>
> err_init:
> mutex_unlock(&devfreq_list_lock);
> -err_devfreq:
> devfreq_remove_device(devfreq);
> - devfreq = NULL;
> + return ERR_PTR(err);
> +
> err_dev:
> + /*
> + * Cleanup path for errors that happen before registration.
> + * Otherwise we rely on devfreq_dev_release.
> + */
> + kfree(devfreq->time_in_state);
> + kfree(devfreq->trans_table);
> kfree(devfreq);
> err_out:
> return ERR_PTR(err);
> }
> EXPORT_SYMBOL(devfreq_add_device);
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH v8 2/6] PM / devfreq: Move more initialization before registration
From: MyungJoo Ham @ 2019-09-25 1:58 UTC (permalink / raw)
To: Leonard Crestez, Kyungmin Park, Matthias Kaehlcke
Cc: Abel Vesa, Saravana Kannan, linux-pm@vger.kernel.org,
Viresh Kumar, Artur Swigon, Krzysztof Kozlowski, Lukasz Luba,
Chanwoo Choi, Alexandre Bailon, NXP Linux Team, Georgi Djakov,
linux-arm-kernel@lists.infradead.org, Jacky Bai
In-Reply-To: <398895f3b4f9ca4b03b47b56dfa25fbd58fd2311.1569319738.git.leonard.crestez@nxp.com>
>In general it is a better to initialize an object before making it
>accessible externally (through device_register).
>
>This makes it possible to avoid relying on locking a partially
>initialized object.
>
>Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Do you object to the general idea of devm_* for device drivers?
or did you find a bug in the memory handling in the code?
The result of this commit still relies on locking anyway.
Cheers,
MyungJoo.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH V9 0/2] mailbox: arm: introduce smc triggered mailbox
From: Peng Fan @ 2019-09-25 2:09 UTC (permalink / raw)
To: robh+dt@kernel.org, mark.rutland@arm.com,
jassisinghbrar@gmail.com, sudeep.holla@arm.com,
andre.przywara@arm.com, f.fainelli@gmail.com
Cc: devicetree@vger.kernel.org, Peng Fan,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, dl-linux-imx
From: Peng Fan <peng.fan@nxp.com>
V9:
- Add Florian's R-b tag in patch 1/2
- Mark arm,func-id as a required property per Andre's comments in patch 1/2.
- Make invoke_smc_mbox_fn as a private entry in a channal per Florian's
comments in pach 2/2
- Include linux/types.h in arm-smccc-mbox.h in patch 2/2
- Drop function_id from arm_smccc_mbox_cmd since func-id is from DT
in patch 2/2/.
Andre,
I have marked arm,func-id as a required property and dropped function-id
from client, please see whether you are happy with the patchset.
Hope we could finalize and get patches land in.
Thanks,
Peng.
V8:
Add missed arm-smccc-mbox.h
V7:
Typo fix
#mbox-cells changed to 0
Add a new header file arm-smccc-mbox.h
Use ARM_SMCCC_IS_64
Andre,
The function_id is still kept in arm_smccc_mbox_cmd, because arm,func-id
property is optional, so clients could pass function_id to mbox driver.
V6:
Switch to per-channel a mbox controller
Drop arm,num-chans, transports, method
Add arm,hvc-mbox compatible
Fix smc/hvc args, drop client id and use correct type.
https://patchwork.kernel.org/cover/11146641/
V5:
yaml fix
https://patchwork.kernel.org/cover/11117741/
V4:
yaml fix for num-chans in patch 1/2.
https://patchwork.kernel.org/cover/11116521/
V3:
Drop interrupt
Introduce transports for mem/reg usage
Add chan-id for mem usage
Convert to yaml format
https://patchwork.kernel.org/cover/11043541/
V2:
This is a modified version from Andre Przywara's patch series
https://lore.kernel.org/patchwork/cover/812997/.
The modification are mostly:
Introduce arm,num-chans
Introduce arm_smccc_mbox_cmd
txdone_poll and txdone_irq are both set to false
arm,func-ids are kept, but as an optional property.
Rewords SCPI to SCMI, because I am trying SCMI over SMC, not SCPI.
Introduce interrupts notification.
[1] is a draft implementation of i.MX8MM SCMI ATF implementation that
use smc as mailbox, power/clk is included, but only part of clk has been
implemented to work with hardware, power domain only supports get name
for now.
The traditional Linux mailbox mechanism uses some kind of dedicated hardware
IP to signal a condition to some other processing unit, typically a dedicated
management processor.
This mailbox feature is used for instance by the SCMI protocol to signal a
request for some action to be taken by the management processor.
However some SoCs does not have a dedicated management core to provide
those services. In order to service TEE and to avoid linux shutdown
power and clock that used by TEE, need let firmware to handle power
and clock, the firmware here is ARM Trusted Firmware that could also
run SCMI service.
The existing SCMI implementation uses a rather flexible shared memory
region to communicate commands and their parameters, it still requires a
mailbox to actually trigger the action.
This patch series provides a Linux mailbox compatible service which uses
smc calls to invoke firmware code, for instance taking care of SCMI requests.
The actual requests are still communicated using the standard SCMI way of
shared memory regions, but a dedicated mailbox hardware IP can be replaced via
this new driver.
This simple driver uses the architected SMC calling convention to trigger
firmware services, also allows for using "HVC" calls to call into hypervisors
or firmware layers running in the EL2 exception level.
Patch 1 contains the device tree binding documentation, patch 2 introduces
the actual mailbox driver.
Please note that this driver just provides a generic mailbox mechanism,
It could support synchronous TX/RX, or synchronous TX with asynchronous
RX. And while providing SCMI services was the reason for this exercise,
this driver is in no way bound to this use case, but can be used generically
where the OS wants to signal a mailbox condition to firmware or a
hypervisor.
Also the driver is in no way meant to replace any existing firmware
interface, but actually to complement existing interfaces.
[1] https://github.com/MrVan/arm-trusted-firmware/tree/scmi
Peng Fan (2):
dt-bindings: mailbox: add binding doc for the ARM SMC/HVC mailbox
mailbox: introduce ARM SMC based mailbox
.../devicetree/bindings/mailbox/arm-smc.yaml | 96 ++++++++++++
drivers/mailbox/Kconfig | 7 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/arm-smc-mailbox.c | 167 +++++++++++++++++++++
include/linux/mailbox/arm-smccc-mbox.h | 20 +++
5 files changed, 292 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.yaml
create mode 100644 drivers/mailbox/arm-smc-mailbox.c
create mode 100644 include/linux/mailbox/arm-smccc-mbox.h
--
2.16.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH V9 1/2] dt-bindings: mailbox: add binding doc for the ARM SMC/HVC mailbox
From: Peng Fan @ 2019-09-25 2:09 UTC (permalink / raw)
To: robh+dt@kernel.org, mark.rutland@arm.com,
jassisinghbrar@gmail.com, sudeep.holla@arm.com,
andre.przywara@arm.com, f.fainelli@gmail.com
Cc: devicetree@vger.kernel.org, Peng Fan,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, dl-linux-imx
In-Reply-To: <1569377224-5755-1-git-send-email-peng.fan@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
The ARM SMC/HVC mailbox binding describes a firmware interface to trigger
actions in software layers running in the EL2 or EL3 exception levels.
The term "ARM" here relates to the SMC instruction as part of the ARM
instruction set, not as a standard endorsed by ARM Ltd.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
.../devicetree/bindings/mailbox/arm-smc.yaml | 96 ++++++++++++++++++++++
1 file changed, 96 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.yaml
diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.yaml b/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
new file mode 100644
index 000000000000..b061954d1678
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/arm-smc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM SMC Mailbox Interface
+
+maintainers:
+ - Peng Fan <peng.fan@nxp.com>
+
+description: |
+ This mailbox uses the ARM smc (secure monitor call) or hvc (hypervisor
+ call) instruction to trigger a mailbox-connected activity in firmware,
+ executing on the very same core as the caller. The value of r0/w0/x0
+ the firmware returns after the smc call is delivered as a received
+ message to the mailbox framework, so synchronous communication can be
+ established. The exact meaning of the action the mailbox triggers as
+ well as the return value is defined by their users and is not subject
+ to this binding.
+
+ One example use case of this mailbox is the SCMI interface, which uses
+ shared memory to transfer commands and parameters, and a mailbox to
+ trigger a function call. This allows SoCs without a separate management
+ processor (or when such a processor is not available or used) to use
+ this standardized interface anyway.
+
+ This binding describes no hardware, but establishes a firmware interface.
+ Upon receiving an SMC using the described SMC function identifier, the
+ firmware is expected to trigger some mailbox connected functionality.
+ The communication follows the ARM SMC calling convention.
+ Firmware expects an SMC function identifier in r0 or w0. The supported
+ identifier are passed from consumers, or listed in the the arm,func-id
+ property as described below. The firmware can return one value in
+ the first SMC result register, it is expected to be an error value,
+ which shall be propagated to the mailbox client.
+
+ Any core which supports the SMC or HVC instruction can be used, as long
+ as a firmware component running in EL3 or EL2 is handling these calls.
+
+properties:
+ compatible:
+ oneOf:
+ - description:
+ For implementations using ARM SMC instruction.
+ const: arm,smc-mbox
+
+ - description:
+ For implementations using ARM HVC instruction.
+ const: arm,hvc-mbox
+
+ "#mbox-cells":
+ const: 0
+
+ arm,func-id:
+ description: |
+ An single 32-bit value specifying the function ID used by the mailbox.
+ The function ID follows the ARM SMC calling convention standard.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+ - compatible
+ - "#mbox-cells"
+ - arm,func-id
+
+examples:
+ - |
+ sram@93f000 {
+ compatible = "mmio-sram";
+ reg = <0x0 0x93f000 0x0 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x0 0x93f000 0x1000>;
+
+ cpu_scp_lpri: scp-shmem@0 {
+ compatible = "arm,scmi-shmem";
+ reg = <0x0 0x200>;
+ };
+ };
+
+ smc_tx_mbox: tx_mbox {
+ #mbox-cells = <0>;
+ compatible = "arm,smc-mbox";
+ arm,func-id = <0xc20000fe>;
+ };
+
+ firmware {
+ scmi {
+ compatible = "arm,scmi";
+ mboxes = <&smc_tx_mbox>;
+ mbox-names = "tx";
+ shmem = <&cpu_scp_lpri>;
+ };
+ };
+
+...
--
2.16.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH V9 2/2] mailbox: introduce ARM SMC based mailbox
From: Peng Fan @ 2019-09-25 2:09 UTC (permalink / raw)
To: robh+dt@kernel.org, mark.rutland@arm.com,
jassisinghbrar@gmail.com, sudeep.holla@arm.com,
andre.przywara@arm.com, f.fainelli@gmail.com
Cc: devicetree@vger.kernel.org, Peng Fan,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, dl-linux-imx
In-Reply-To: <1569377224-5755-1-git-send-email-peng.fan@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
This mailbox driver implements a mailbox which signals transmitted data
via an ARM smc (secure monitor call) instruction. The mailbox receiver
is implemented in firmware and can synchronously return data when it
returns execution to the non-secure world again.
An asynchronous receive path is not implemented.
This allows the usage of a mailbox to trigger firmware actions on SoCs
which either don't have a separate management processor or on which such
a core is not available. A user of this mailbox could be the SCP
interface.
Modified from Andre Przywara's v2 patch
https://lore.kernel.org/patchwork/patch/812999/
Cc: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/mailbox/Kconfig | 7 ++
drivers/mailbox/Makefile | 2 +
drivers/mailbox/arm-smc-mailbox.c | 167 +++++++++++++++++++++++++++++++++
include/linux/mailbox/arm-smccc-mbox.h | 20 ++++
4 files changed, 196 insertions(+)
create mode 100644 drivers/mailbox/arm-smc-mailbox.c
create mode 100644 include/linux/mailbox/arm-smccc-mbox.h
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index ab4eb750bbdd..7707ee26251a 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -16,6 +16,13 @@ config ARM_MHU
The controller has 3 mailbox channels, the last of which can be
used in Secure mode only.
+config ARM_SMC_MBOX
+ tristate "Generic ARM smc mailbox"
+ depends on OF && HAVE_ARM_SMCCC
+ help
+ Generic mailbox driver which uses ARM smc calls to call into
+ firmware for triggering mailboxes.
+
config IMX_MBOX
tristate "i.MX Mailbox"
depends on ARCH_MXC || COMPILE_TEST
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index c22fad6f696b..93918a84c91b 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -7,6 +7,8 @@ obj-$(CONFIG_MAILBOX_TEST) += mailbox-test.o
obj-$(CONFIG_ARM_MHU) += arm_mhu.o
+obj-$(CONFIG_ARM_SMC_MBOX) += arm-smc-mailbox.o
+
obj-$(CONFIG_IMX_MBOX) += imx-mailbox.o
obj-$(CONFIG_ARMADA_37XX_RWTM_MBOX) += armada-37xx-rwtm-mailbox.o
diff --git a/drivers/mailbox/arm-smc-mailbox.c b/drivers/mailbox/arm-smc-mailbox.c
new file mode 100644
index 000000000000..6f0b5fd6ad1b
--- /dev/null
+++ b/drivers/mailbox/arm-smc-mailbox.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016,2017 ARM Ltd.
+ * Copyright 2019 NXP
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/interrupt.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox/arm-smccc-mbox.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+typedef unsigned long (smc_mbox_fn)(unsigned int, unsigned long,
+ unsigned long, unsigned long,
+ unsigned long, unsigned long,
+ unsigned long);
+
+struct arm_smc_chan_data {
+ unsigned int function_id;
+ smc_mbox_fn *invoke_smc_mbox_fn;
+};
+
+static int arm_smc_send_data(struct mbox_chan *link, void *data)
+{
+ struct arm_smc_chan_data *chan_data = link->con_priv;
+ struct arm_smccc_mbox_cmd *cmd = data;
+ unsigned long ret;
+
+ if (ARM_SMCCC_IS_64(chan_data->function_id)) {
+ ret = chan_data->invoke_smc_mbox_fn(chan_data->function_id,
+ cmd->args_smccc64[0],
+ cmd->args_smccc64[1],
+ cmd->args_smccc64[2],
+ cmd->args_smccc64[3],
+ cmd->args_smccc64[4],
+ cmd->args_smccc64[5]);
+ } else {
+ ret = chan_data->invoke_smc_mbox_fn(chan_data->function_id,
+ cmd->args_smccc32[0],
+ cmd->args_smccc32[1],
+ cmd->args_smccc32[2],
+ cmd->args_smccc32[3],
+ cmd->args_smccc32[4],
+ cmd->args_smccc32[5]);
+ }
+
+ mbox_chan_received_data(link, (void *)ret);
+
+ return 0;
+}
+
+static unsigned long __invoke_fn_hvc(unsigned int function_id,
+ unsigned long arg0, unsigned long arg1,
+ unsigned long arg2, unsigned long arg3,
+ unsigned long arg4, unsigned long arg5)
+{
+ struct arm_smccc_res res;
+
+ arm_smccc_hvc(function_id, arg0, arg1, arg2, arg3, arg4,
+ arg5, 0, &res);
+ return res.a0;
+}
+
+static unsigned long __invoke_fn_smc(unsigned int function_id,
+ unsigned long arg0, unsigned long arg1,
+ unsigned long arg2, unsigned long arg3,
+ unsigned long arg4, unsigned long arg5)
+{
+ struct arm_smccc_res res;
+
+ arm_smccc_smc(function_id, arg0, arg1, arg2, arg3, arg4,
+ arg5, 0, &res);
+ return res.a0;
+}
+
+static const struct mbox_chan_ops arm_smc_mbox_chan_ops = {
+ .send_data = arm_smc_send_data,
+};
+
+static struct mbox_chan *
+arm_smc_mbox_of_xlate(struct mbox_controller *mbox,
+ const struct of_phandle_args *sp)
+{
+ return mbox->chans;
+}
+
+static int arm_smc_mbox_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mbox_controller *mbox;
+ struct arm_smc_chan_data *chan_data;
+ int ret;
+
+ mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+ if (!mbox)
+ return -ENOMEM;
+
+ mbox->of_xlate = arm_smc_mbox_of_xlate;
+ mbox->num_chans = 1;
+ mbox->chans = devm_kzalloc(dev, sizeof(*mbox->chans), GFP_KERNEL);
+ if (!mbox->chans)
+ return -ENOMEM;
+
+ chan_data = devm_kzalloc(dev, sizeof(*chan_data), GFP_KERNEL);
+ if (!chan_data)
+ return -ENOMEM;
+
+ ret = of_property_read_u32(dev->of_node, "arm,func-id",
+ &chan_data->function_id);
+ if (ret)
+ return ret;
+
+ if (of_device_is_compatible(dev->of_node, "arm,smc-mbox"))
+ chan_data->invoke_smc_mbox_fn = __invoke_fn_smc;
+ else
+ chan_data->invoke_smc_mbox_fn = __invoke_fn_hvc;
+
+
+ mbox->chans->con_priv = chan_data;
+
+ mbox->txdone_poll = false;
+ mbox->txdone_irq = false;
+ mbox->ops = &arm_smc_mbox_chan_ops;
+ mbox->dev = dev;
+
+ platform_set_drvdata(pdev, mbox);
+
+ ret = devm_mbox_controller_register(dev, mbox);
+ if (ret)
+ return ret;
+
+ dev_info(dev, "ARM SMC mailbox enabled.\n");
+
+ return ret;
+}
+
+static int arm_smc_mbox_remove(struct platform_device *pdev)
+{
+ struct mbox_controller *mbox = platform_get_drvdata(pdev);
+
+ mbox_controller_unregister(mbox);
+ return 0;
+}
+
+static const struct of_device_id arm_smc_mbox_of_match[] = {
+ { .compatible = "arm,smc-mbox", },
+ { .compatible = "arm,hvc-mbox", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, arm_smc_mbox_of_match);
+
+static struct platform_driver arm_smc_mbox_driver = {
+ .driver = {
+ .name = "arm-smc-mbox",
+ .of_match_table = arm_smc_mbox_of_match,
+ },
+ .probe = arm_smc_mbox_probe,
+ .remove = arm_smc_mbox_remove,
+};
+module_platform_driver(arm_smc_mbox_driver);
+
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_DESCRIPTION("Generic ARM smc mailbox driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mailbox/arm-smccc-mbox.h b/include/linux/mailbox/arm-smccc-mbox.h
new file mode 100644
index 000000000000..d35fb89a77f5
--- /dev/null
+++ b/include/linux/mailbox/arm-smccc-mbox.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_ARM_SMCCC_MBOX_H_
+#define _LINUX_ARM_SMCCC_MBOX_H_
+
+#include <linux/types.h>
+
+/**
+ * struct arm_smccc_mbox_cmd - ARM SMCCC message structure
+ * @args_smccc32/64: actual usage of registers is up to the protocol
+ * (within the SMCCC limits)
+ */
+struct arm_smccc_mbox_cmd {
+ union {
+ u32 args_smccc32[6];
+ u64 args_smccc64[6];
+ };
+};
+
+#endif /* _LINUX_ARM_SMCCC_MBOX_H_ */
--
2.16.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v8 5/6] PM / devfreq: Add PM QoS support
From: Chanwoo Choi @ 2019-09-25 2:15 UTC (permalink / raw)
To: Leonard Crestez, MyungJoo Ham, Kyungmin Park, Matthias Kaehlcke
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, NXP Linux Team, Krzysztof Kozlowski, Lukasz Luba,
Alexandre Bailon, Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <58fdd2c226a4e76a3d9427baab7dd5c23af842ab.1569319738.git.leonard.crestez@nxp.com>
On 19. 9. 24. 오후 7:11, Leonard Crestez wrote:
> Register notifiers with the PM QoS framework in order to respond to
> requests for DEV_PM_QOS_MIN_FREQUENCY and DEV_PM_QOS_MAX_FREQUENCY.
>
> No notifiers are added by this patch but PM QoS constraints can be
> imposed externally (for example from other devices).
>
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> drivers/devfreq/devfreq.c | 75 +++++++++++++++++++++++++++++++++++++++
> include/linux/devfreq.h | 5 +++
> 2 files changed, 80 insertions(+)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index eee403e70c84..784f3e40536a 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -22,15 +22,18 @@
> #include <linux/platform_device.h>
> #include <linux/list.h>
> #include <linux/printk.h>
> #include <linux/hrtimer.h>
> #include <linux/of.h>
> +#include <linux/pm_qos.h>
> #include "governor.h"
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/devfreq.h>
>
> +#define HZ_PER_KHZ 1000
> +
> static struct class *devfreq_class;
>
> /*
> * devfreq core provides delayed work based load monitoring helper
> * functions. Governors can use these or can implement their own
> @@ -109,10 +112,11 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq)
> static void get_freq_range(struct devfreq *devfreq,
> unsigned long *min_freq,
> unsigned long *max_freq)
> {
> unsigned long *freq_table = devfreq->profile->freq_table;
> + unsigned long qos_min_freq, qos_max_freq;
>
> lockdep_assert_held(&devfreq->lock);
>
> /*
> * Init min/max frequency from freq table.
> @@ -125,10 +129,18 @@ static void get_freq_range(struct devfreq *devfreq,
> } else {
> *min_freq = freq_table[devfreq->profile->max_state - 1];
> *max_freq = freq_table[0];
> }
>
> + /* constraints from PM QoS */
As I commented on patch4,
'constraints' -> 'Constraint' because first verb have to be used
as the sigular verbs.
I prefer to use following comments:
/* Constraint minimum/maximum frequency from PM QoS constraints */
> + qos_min_freq = dev_pm_qos_read_value(devfreq->dev.parent,
> + DEV_PM_QOS_MIN_FREQUENCY);
> + qos_max_freq = dev_pm_qos_read_value(devfreq->dev.parent,
> + DEV_PM_QOS_MIN_FREQUENCY);
> + *min_freq = max(*min_freq, HZ_PER_KHZ * qos_min_freq);
> + *max_freq = min(*max_freq, HZ_PER_KHZ * qos_max_freq);
> +
> /* constraints from sysfs */
> *min_freq = max(*min_freq, devfreq->min_freq);
> *max_freq = min(*max_freq, devfreq->max_freq);
>
> /* constraints from OPP interface */
> @@ -606,10 +618,49 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
> mutex_unlock(&devfreq->lock);
>
> return ret;
> }
>
> +/**
> + * qos_notifier_call() - Common handler for QoS constraints.
> + * @devfreq: the devfreq instance.
> + */
> +static int qos_notifier_call(struct devfreq *devfreq)
> +{
> + int err;
> +
> + mutex_lock(&devfreq->lock);
> + err = update_devfreq(devfreq);
> + mutex_unlock(&devfreq->lock);
> + if (err)
> + dev_err(devfreq->dev.parent,
> + "failed to update frequency for PM QoS constraints (%d)\n",
Is it not over 80 char?
> + err);
> +
> + return NOTIFY_OK;
> +}
> +
> +/**
> + * qos_min_notifier_call() - Callback for QoS min_freq changes.
> + * @nb: Should be devfreq->nb_min
> + */
> +static int qos_min_notifier_call(struct notifier_block *nb,
> + unsigned long val, void *ptr)
> +{
> + return qos_notifier_call(container_of(nb, struct devfreq, nb_min));
> +}
> +
> +/**
> + * qos_max_notifier_call() - Callback for QoS max_freq changes.
> + * @nb: Should be devfreq->nb_max
> + */
> +static int qos_max_notifier_call(struct notifier_block *nb,
> + unsigned long val, void *ptr)
> +{
> + return qos_notifier_call(container_of(nb, struct devfreq, nb_max));
> +}
> +
> /**
> * devfreq_dev_release() - Callback for struct device to release the device.
> * @dev: the devfreq device
> *
> * Remove devfreq from the list and release its resources.
> @@ -620,10 +671,15 @@ static void devfreq_dev_release(struct device *dev)
>
> mutex_lock(&devfreq_list_lock);
> list_del(&devfreq->node);
> mutex_unlock(&devfreq_list_lock);
>
> + dev_pm_qos_remove_notifier(devfreq->dev.parent, &devfreq->nb_max,
> + DEV_PM_QOS_MAX_FREQUENCY);
> + dev_pm_qos_remove_notifier(devfreq->dev.parent, &devfreq->nb_min,
> + DEV_PM_QOS_MIN_FREQUENCY);
> +
Just print error with dev_err() without stopping the release step.
I prefer to handle the return value if kernel API provides
the error code.
> if (devfreq->profile->exit)
> devfreq->profile->exit(devfreq->dev.parent);
>
> kfree(devfreq->time_in_state);
> kfree(devfreq->trans_table);
> @@ -733,10 +789,28 @@ struct devfreq *devfreq_add_device(struct device *dev,
> if (err) {
> put_device(&devfreq->dev);
> goto err_out;
> }
>
> + /*
> + * Register notifiers for updates to min/max_freq after device is
> + * initialized (and we can handle notifications) but before the
> + * governor is started (which should do an initial enforcement of
> + * constraints).
> + */
My previous comment is not enough why I prefer to remove it. Sorry.
Actually, until now, the devfreq_add_device() don't have the detailed
comments because the line code is not too long. But, at the present time,
devfreq_add_device() is too long. It means that the detailed comment
are necessary.
So, I'll add the detailed comment for each step of devfreq_add_device()
on separate patch to keep the same style. I'll send the patch to you
for the review.
> + devfreq->nb_min.notifier_call = qos_min_notifier_call;
> + err = dev_pm_qos_add_notifier(devfreq->dev.parent, &devfreq->nb_min,
> + DEV_PM_QOS_MIN_FREQUENCY);
> + if (err)
> + goto err_devfreq;
> +
> + devfreq->nb_max.notifier_call = qos_max_notifier_call;
> + err = dev_pm_qos_add_notifier(devfreq->dev.parent, &devfreq->nb_max,
> + DEV_PM_QOS_MAX_FREQUENCY);
> + if (err)
> + goto err_devfreq;
> +
> mutex_lock(&devfreq_list_lock);
>
> governor = try_then_request_governor(devfreq->governor_name);
> if (IS_ERR(governor)) {
> dev_err(dev, "%s: Unable to find governor for the device\n",
> @@ -760,10 +834,11 @@ struct devfreq *devfreq_add_device(struct device *dev,
>
> return devfreq;
>
> err_init:
> mutex_unlock(&devfreq_list_lock);
> +err_devfreq:
> devfreq_remove_device(devfreq);
> return ERR_PTR(err);
>
> err_dev:
> /*
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index c3cbc15fdf08..dac0dffeabb4 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -134,10 +134,12 @@ struct devfreq_dev_profile {
> * @total_trans: Number of devfreq transitions
> * @trans_table: Statistics of devfreq transitions
> * @time_in_state: Statistics of devfreq states
> * @last_stat_updated: The last time stat updated
> * @transition_notifier_list: list head of DEVFREQ_TRANSITION_NOTIFIER notifier
> + * @nb_min: Notifier block for DEV_PM_QOS_MIN_FREQUENCY
> + * @nb_max: Notifier block for DEV_PM_QOS_MAX_FREQUENCY
> *
> * This structure stores the devfreq information for a give device.
> *
> * Note that when a governor accesses entries in struct devfreq in its
> * functions except for the context of callbacks defined in struct
> @@ -176,10 +178,13 @@ struct devfreq {
> unsigned int *trans_table;
> unsigned long *time_in_state;
> unsigned long last_stat_updated;
>
> struct srcu_notifier_head transition_notifier_list;
> +
> + struct notifier_block nb_min;
> + struct notifier_block nb_max;
> };
>
> struct devfreq_freqs {
> unsigned long old;
> unsigned long new;
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 5/6] PM / devfreq: Add PM QoS support
From: Chanwoo Choi @ 2019-09-25 2:17 UTC (permalink / raw)
To: Leonard Crestez, Matthias Kaehlcke
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan,
linux-pm@vger.kernel.org, Viresh Kumar, dl-linux-imx,
Krzysztof Kozlowski, Lukasz Luba, Kyungmin Park, MyungJoo Ham,
Alexandre Bailon, Georgi Djakov,
linux-arm-kernel@lists.infradead.org, Jacky Bai
In-Reply-To: <VI1PR04MB7023034123415F86CC045CCDEE840@VI1PR04MB7023.eurprd04.prod.outlook.com>
On 19. 9. 25. 오전 4:22, Leonard Crestez wrote:
> On 24.09.2019 22:11, Matthias Kaehlcke wrote:
>> On Tue, Sep 24, 2019 at 01:11:29PM +0300, Leonard Crestez wrote:
>>> Register notifiers with the PM QoS framework in order to respond to
>>> requests for DEV_PM_QOS_MIN_FREQUENCY and DEV_PM_QOS_MAX_FREQUENCY.
>>>
>>> No notifiers are added by this patch but PM QoS constraints can be
>>> imposed externally (for example from other devices).
>>>
>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
>>> ---
>>> drivers/devfreq/devfreq.c | 75 +++++++++++++++++++++++++++++++++++++++
>>> include/linux/devfreq.h | 5 +++
>>> 2 files changed, 80 insertions(+)
>>>
>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>> index eee403e70c84..784f3e40536a 100644
>>> --- a/drivers/devfreq/devfreq.c
>>> +++ b/drivers/devfreq/devfreq.c
>>> @@ -22,15 +22,18 @@
>>> #include <linux/platform_device.h>
>>> #include <linux/list.h>
>>> #include <linux/printk.h>
>>> #include <linux/hrtimer.h>
>>> #include <linux/of.h>
>>> +#include <linux/pm_qos.h>
>>> #include "governor.h"
>>>
>>> #define CREATE_TRACE_POINTS
>>> #include <trace/events/devfreq.h>
>>>
>>> +#define HZ_PER_KHZ 1000
>>> +
>>> static struct class *devfreq_class;
>>>
>>> /*
>>> * devfreq core provides delayed work based load monitoring helper
>>> * functions. Governors can use these or can implement their own
>>> @@ -109,10 +112,11 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq)
>>> static void get_freq_range(struct devfreq *devfreq,
>>> unsigned long *min_freq,
>>> unsigned long *max_freq)
>>> {
>>> unsigned long *freq_table = devfreq->profile->freq_table;
>>> + unsigned long qos_min_freq, qos_max_freq;
>>>
>>> lockdep_assert_held(&devfreq->lock);
>>>
>>> /*
>>> * Init min/max frequency from freq table.
>>> @@ -125,10 +129,18 @@ static void get_freq_range(struct devfreq *devfreq,
>>> } else {
>>> *min_freq = freq_table[devfreq->profile->max_state - 1];
>>> *max_freq = freq_table[0];
>>> }
>>>
>>> + /* constraints from PM QoS */
>>> + qos_min_freq = dev_pm_qos_read_value(devfreq->dev.parent,
>>> + DEV_PM_QOS_MIN_FREQUENCY);
>>> + qos_max_freq = dev_pm_qos_read_value(devfreq->dev.parent,
>>> + DEV_PM_QOS_MIN_FREQUENCY); >
>> This needs to be DEV_PM_QOS_MAX_FREQUENCY. I missed this in the earlier
>> reviews, but stumbled across it when testing.
>>
>> !Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
>
> I broke it in v8 while processing comments. Thanks for catching it.
Did you test this patchset with v8?
Maybe it is not working with this mistake.
>
> --
> Regards,
> Leonard
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH] pwm: pwm-imx27: Use 'dev' instead of dereferencing it repeatedly
From: Anson Huang @ 2019-09-25 2:36 UTC (permalink / raw)
To: David Laight, thierry.reding@gmail.com, shawnguo@kernel.org,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
linux-pwm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <37b2481ac9094f27bf21325e2770abdb@AcuMS.aculab.com>
Hi, David
> Subject: RE: [PATCH] pwm: pwm-imx27: Use 'dev' instead of dereferencing it
> repeatedly
>
> From: Anson Huang
> > Sent: 24 September 2019 11:03
> > Hi, David
> >
> > > Subject: RE: [PATCH] pwm: pwm-imx27: Use 'dev' instead of
> > > dereferencing it repeatedly
> > >
> > > From: Anson Huang
> > > > Sent: 24 September 2019 10:00
> > > > Add helper variable dev = &pdev->dev to simply the code.
> > > >
> ...
> > > > static int pwm_imx27_probe(struct platform_device *pdev) {
> > > > + struct device *dev = &pdev->dev;
> > > > struct pwm_imx27_chip *imx;
> > > >
> > > > - imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
> > > > + imx = devm_kzalloc(dev, sizeof(*imx), GFP_KERNEL);
> ...
> > > Hopefully the compiler will optimise this back otherwise you've
> > > added another local variable which may cause spilling to stack.
> > > For a setup function it probably doesn't matter, but in general it
> > > might have a small negative performance impact.
> > >
> > > In any case this doesn't shorten any lines enough to remove
> > > line-wrap and using &pdev->dev is really one less variable to
> > > mentally track when reading the code.
> >
> > Do we know which compiler will optimize this? I saw many of the
> > patches doing this to avoid a lot of dereference, I understand it does
> > NOT save lines, but my intention is to avoid dereference which might save
> some instructions.
> >
> > I thought saving instructions is more important. So now there are
> > different opinion about doing this?
>
> Remember &pdev->dev is just 'pdev + constant'.
> Assuming 'pdev' is held in a callee saved register (which you want it to be)
> then to access
> dev->foo the compiler can remember the constant and use an offset from
> dev->'pdev' instead of
> an extra 'dev' variable.
> On most modern ABI the first function call arguments are passed in registers.
> So an add instruction (probably lea) can be used to add the constant offset
> at the same time as the value is moved into the argument register.
>
> However your extra variable could easily get spilled out to the stack.
> So you get an extra memory read rather than (at most) an extra 'add'
> instruction.
>
> Even if pdev->dev were a pointer, repeatedly reading it from pdev->dev
> could easily generate better code than having an extra variable that would
> mean the value was repeatedly read from the stack.
Thanks for detail education about it, please ignore these patches.
Thanks,
Anson
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 6/6] PM / devfreq: Use PM QoS for sysfs min/max_freq
From: Chanwoo Choi @ 2019-09-25 2:41 UTC (permalink / raw)
To: Leonard Crestez, MyungJoo Ham, Kyungmin Park, Matthias Kaehlcke
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, NXP Linux Team, Krzysztof Kozlowski, Lukasz Luba,
Alexandre Bailon, Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <3eb8a1d1e7d738f7599d54348d6b474196581693.1569319738.git.leonard.crestez@nxp.com>
On 19. 9. 24. 오후 7:11, Leonard Crestez wrote:
> Switch the handling of min_freq and max_freq from sysfs to use the
> dev_pm_qos_request interface.
>
> Since PM QoS handles frequencies as kHz this change reduces the
> precision of min_freq and max_freq. This shouldn't introduce problems
> because frequencies which are not an integer number of kHz are likely
> not an integer number of Hz either.
>
> Try to ensure compatibility by rounding min values down and rounding
> max values up.
>
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> drivers/devfreq/devfreq.c | 46 ++++++++++++++++++++++++---------------
> include/linux/devfreq.h | 9 ++++----
> 2 files changed, 33 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 784f3e40536a..8bb7efd821ab 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -137,14 +137,10 @@ static void get_freq_range(struct devfreq *devfreq,
> qos_max_freq = dev_pm_qos_read_value(devfreq->dev.parent,
> DEV_PM_QOS_MIN_FREQUENCY);
> *min_freq = max(*min_freq, HZ_PER_KHZ * qos_min_freq);
> *max_freq = min(*max_freq, HZ_PER_KHZ * qos_max_freq);
>
> - /* constraints from sysfs */
> - *min_freq = max(*min_freq, devfreq->min_freq);
> - *max_freq = min(*max_freq, devfreq->max_freq);
> -
> /* constraints from OPP interface */
> *min_freq = max(*min_freq, devfreq->scaling_min_freq);
> /* scaling_max_freq can be zero on error */
> if (devfreq->scaling_max_freq)
> *max_freq = min(*max_freq, devfreq->scaling_max_freq);
> @@ -679,10 +675,12 @@ static void devfreq_dev_release(struct device *dev)
> DEV_PM_QOS_MIN_FREQUENCY);
>
> if (devfreq->profile->exit)
> devfreq->profile->exit(devfreq->dev.parent);
>
> + dev_pm_qos_remove_request(&devfreq->user_max_freq_req);
> + dev_pm_qos_remove_request(&devfreq->user_min_freq_req);
Please check the return value if error happen, just print the err with dev_err()
without stopping the release steps.
> kfree(devfreq->time_in_state);
> kfree(devfreq->trans_table);
> mutex_destroy(&devfreq->lock);
> kfree(devfreq);
> }
> @@ -747,18 +745,25 @@ struct devfreq *devfreq_add_device(struct device *dev,
> devfreq->scaling_min_freq = find_available_min_freq(devfreq);
> if (!devfreq->scaling_min_freq) {
> err = -EINVAL;
> goto err_dev;
> }
> - devfreq->min_freq = devfreq->scaling_min_freq;
>
> devfreq->scaling_max_freq = find_available_max_freq(devfreq);
> if (!devfreq->scaling_max_freq) {
> err = -EINVAL;
> goto err_dev;
> }
> - devfreq->max_freq = devfreq->scaling_max_freq;
> +
> + err = dev_pm_qos_add_request(dev, &devfreq->user_min_freq_req,
> + DEV_PM_QOS_MIN_FREQUENCY, 0);
> + if (err < 0)
> + goto err_dev;
> + err = dev_pm_qos_add_request(dev, &devfreq->user_max_freq_req,
> + DEV_PM_QOS_MAX_FREQUENCY, S32_MAX);
> + if (err < 0)
> + goto err_dev;
>
> devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
> atomic_set(&devfreq->suspend_count, 0);
>
> devfreq->trans_table = kzalloc(
> @@ -843,10 +848,14 @@ struct devfreq *devfreq_add_device(struct device *dev,
> err_dev:
> /*
> * Cleanup path for errors that happen before registration.
> * Otherwise we rely on devfreq_dev_release.
> */
> + if (dev_pm_qos_request_active(&devfreq->user_max_freq_req))
> + dev_pm_qos_remove_request(&devfreq->user_max_freq_req);
Please check the return value if error happen, just print the err with dev_err()
without stopping the release steps.
dev_err(... "failed to remove request of DEV_PM_QOS_MAX_FREQUENCY\n");
> + if (dev_pm_qos_request_active(&devfreq->user_min_freq_req))
> + dev_pm_qos_remove_request(&devfreq->user_min_freq_req);
dev_err(... "failed to remove request of DEV_PM_QOS_MIN_FREQUENCY\n");
> kfree(devfreq->time_in_state);
> kfree(devfreq->trans_table);
> kfree(devfreq);
> err_out:
> return ERR_PTR(err);
> @@ -1407,14 +1416,15 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
>
> ret = sscanf(buf, "%lu", &value);
> if (ret != 1)
> return -EINVAL;
>
> - mutex_lock(&df->lock);
> - df->min_freq = value;
> - update_devfreq(df);
> - mutex_unlock(&df->lock);
> + /* round down to kHz for PM QoS */
I prefer more detailed description as following:
/*
* Round down to KHz to decide the proper minimum frequency
* which is closed to user request.
*/
> + ret = dev_pm_qos_update_request(&df->user_min_freq_req,
> + value / HZ_PER_KHZ);
> + if (ret < 0)
> + return ret;
>
> return count;
> }
>
> static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
> @@ -1439,19 +1449,19 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
>
> ret = sscanf(buf, "%lu", &value);
> if (ret != 1)
> return -EINVAL;
>
> - mutex_lock(&df->lock);
> -
> - /* Interpret zero as "don't care" */
> - if (!value)
> - value = ULONG_MAX;
> + /* round up to kHz for PM QoS and interpret zero as "don't care" */
I think that "don't care" comment style is not good.
I referred to the Documentation/ABI/testing/sysfs-class-devfreq file.
I prefer more detailed description as following:
/*
* Round up to KHz to decide the proper maximum frequency
* which is closed to user request. If value is zero,
* the user does not care.
*/
> + if (value)
> + value = DIV_ROUND_UP(value, HZ_PER_KHZ);
> + else
> + value = S32_MAX;
>
> - df->max_freq = value;
> - update_devfreq(df);
> - mutex_unlock(&df->lock);
> + ret = dev_pm_qos_update_request(&df->user_max_freq_req, value);
> + if (ret < 0)
> + return ret;
>
> return count;
> }
> static DEVICE_ATTR_RW(min_freq);
>
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index dac0dffeabb4..7849fe4c666d 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -11,10 +11,11 @@
> #define __LINUX_DEVFREQ_H__
>
> #include <linux/device.h>
> #include <linux/notifier.h>
> #include <linux/pm_opp.h>
> +#include <linux/pm_qos.h>
>
> #define DEVFREQ_NAME_LEN 16
>
> /* DEVFREQ governor name */
> #define DEVFREQ_GOV_SIMPLE_ONDEMAND "simple_ondemand"
> @@ -121,12 +122,12 @@ struct devfreq_dev_profile {
> * devfreq.nb to the corresponding register notifier call chain.
> * @work: delayed work for load monitoring.
> * @previous_freq: previously configured frequency value.
> * @data: Private data of the governor. The devfreq framework does not
> * touch this.
> - * @min_freq: Limit minimum frequency requested by user (0: none)
> - * @max_freq: Limit maximum frequency requested by user (0: none)
> + * @user_min_freq_req: PM QoS min frequency request from user (via sysfs)
min -> minimum and then remove parenthesis as following:
PM QoS minimum frequency request by user via sysfs
> + * @user_max_freq_req: PM QoS max frequency request from user (via sysfs)
ditto. max -> maximum
PM QoS maximum frequency request by user via sysfs
> * @scaling_min_freq: Limit minimum frequency requested by OPP interface
> * @scaling_max_freq: Limit maximum frequency requested by OPP interface
> * @stop_polling: devfreq polling status of a device.
> * @suspend_freq: frequency of a device set during suspend phase.
> * @resume_freq: frequency of a device set in resume phase.
> @@ -161,12 +162,12 @@ struct devfreq {
> unsigned long previous_freq;
> struct devfreq_dev_status last_status;
>
> void *data; /* private data for governors */
>
> - unsigned long min_freq;
> - unsigned long max_freq;
> + struct dev_pm_qos_request user_min_freq_req;
> + struct dev_pm_qos_request user_max_freq_req;
> unsigned long scaling_min_freq;
> unsigned long scaling_max_freq;
> bool stop_polling;
>
> unsigned long suspend_freq;
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFCv4 0/7] interconnect: Add imx support via devfreq
From: Georgi Djakov @ 2019-09-25 2:37 UTC (permalink / raw)
To: Leonard Crestez, Rob Herring, Mark Rutland,
devicetree@vger.kernel.org
Cc: Aisheng Dong, Artur Świgoń, Saravana Kannan,
linux-pm@vger.kernel.org, Stephen Boyd, Viresh Kumar,
Michael Turquette, Krzysztof Kozlowski, Chanwoo Choi,
Kyungmin Park, MyungJoo Ham, Alexandre Bailon,
kernel@pengutronix.de, Fabio Estevam, Shawn Guo,
linux-arm-kernel@lists.infradead.org, dl-linux-imx
In-Reply-To: <VI1PR04MB702373BE28E4404C7F2CE75FEE8C0@VI1PR04MB7023.eurprd04.prod.outlook.com>
Hi Leonard,
On 9/16/19 05:34, Leonard Crestez wrote:
> On 23.08.2019 17:37, Leonard Crestez wrote:
>> This series add imx support for interconnect via devfreq: the ICC
>> framework is used to aggregate requests from devices and then those are
>> converted to DEV_PM_QOS_MIN_FREQUENCY requests for devfreq.
>>
>> Since there is no single devicetree node that can represent the "interconnect"
>> new API is added to allow individual devfreq nodes to act as parsing proxies
>> all mapping to a single soc-level icc provider. This is still RFC
>> because of this
>
> Any comments? I made a lot of changes relative to previous versions,
> most of them solely to avoid adding a virtual node in DT bindings.
>
> The only current interconnect provider implementation is for qcom and it
> uses a firmware node as the provider node (with #interconnect-cells).
> However there is no obvious equivalent of that for imx and many other SOCs.
Not sure if it will help, but have you seen the qcs404 interconnect driver?
There is also mt8183 interconnect provider driver on LKML.
> On imx there are multiple pieces of scalable fabric which can be defined
> in DT as devfreq devices and it sort of makes sense to add
> #interconnect-cells to those. However when it comes to describing the
> SOC interconnect graph it's much more convenient to have a single
> per-SOC platform driver.
Is all the NoC configuration done only by ATF? Are there any NoC related memory
mapped registers?
Thanks,
Georgi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v9 0/3] fix double page fault on arm64
From: Jia He @ 2019-09-25 2:59 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Mark Rutland, James Morse,
Marc Zyngier, Matthew Wilcox, Kirill A. Shutemov,
linux-arm-kernel, linux-kernel, linux-mm, Suzuki Poulose
Cc: Ralph Campbell, Jia He, Anshuman Khandual, Alex Van Brunt,
Kaly Xin, Jérôme Glisse, Punit Agrawal, hejianet,
Andrew Morton, nd, Robin Murphy, Thomas Gleixner
When we tested pmdk unit test vmmalloc_fork TEST1 in arm64 guest, there
will be a double page fault in __copy_from_user_inatomic of cow_user_page.
As told by Catalin: "On arm64 without hardware Access Flag, copying from
user will fail because the pte is old and cannot be marked young. So we
always end up with zeroed page after fork() + CoW for pfn mappings. we
don't always have a hardware-managed access flag on arm64."
Changes
v9: refactor cow_user_page for indention optimization (Catalin)
hold the ptl longer (Catalin)
v8: change cow_user_page's return type (Matthew)
v7: s/pte_spinlock/pte_offset_map_lock (Kirill)
v6: fix error case of returning with spinlock taken (Catalin)
move kmap_atomic to avoid handling kunmap_atomic
v5: handle the case correctly when !pte_same
fix kbuild test failed
v4: introduce cpu_has_hw_af (Suzuki)
bail out if !pte_same (Kirill)
v3: add vmf->ptl lock/unlock (Kirill A. Shutemov)
add arch_faults_on_old_pte (Matthew, Catalin)
v2: remove FAULT_FLAG_WRITE when setting pte access flag (Catalin)
Jia He (3):
arm64: cpufeature: introduce helper cpu_has_hw_af()
arm64: mm: implement arch_faults_on_old_pte() on arm64
mm: fix double page fault on arm64 if PTE_AF is cleared
arch/arm64/include/asm/cpufeature.h | 10 +++
arch/arm64/include/asm/pgtable.h | 14 ++++
mm/memory.c | 99 ++++++++++++++++++++++++-----
3 files changed, 108 insertions(+), 15 deletions(-)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v9 1/3] arm64: cpufeature: introduce helper cpu_has_hw_af()
From: Jia He @ 2019-09-25 2:59 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Mark Rutland, James Morse,
Marc Zyngier, Matthew Wilcox, Kirill A. Shutemov,
linux-arm-kernel, linux-kernel, linux-mm, Suzuki Poulose
Cc: Ralph Campbell, Jia He, Anshuman Khandual, Alex Van Brunt,
Kaly Xin, Jérôme Glisse, Punit Agrawal, hejianet,
Andrew Morton, nd, Robin Murphy, Thomas Gleixner
In-Reply-To: <20190925025922.176362-1-justin.he@arm.com>
We unconditionally set the HW_AFDBM capability and only enable it on
CPUs which really have the feature. But sometimes we need to know
whether this cpu has the capability of HW AF. So decouple AF from
DBM by new helper cpu_has_hw_af().
Signed-off-by: Jia He <justin.he@arm.com>
Suggested-by: Suzuki Poulose <Suzuki.Poulose@arm.com>
Reported-by: kbuild test robot <lkp@intel.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/cpufeature.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index c96ffa4722d3..c2e3abd39faa 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -667,6 +667,16 @@ static inline u32 id_aa64mmfr0_parange_to_phys_shift(int parange)
default: return CONFIG_ARM64_PA_BITS;
}
}
+
+/* Check whether hardware update of the Access flag is supported */
+static inline bool cpu_has_hw_af(void)
+{
+ if (IS_ENABLED(CONFIG_ARM64_HW_AFDBM))
+ return read_cpuid(ID_AA64MMFR1_EL1) & 0xf;
+
+ return false;
+}
+
#endif /* __ASSEMBLY__ */
#endif
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
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