iommu/msm: Add DT adaptation and generic bindings support
Subsystems (4)
v2 cover iommu/msm: Add DT adaptation and generic bindings support
The msm_iommu.c driver currently works based on platform data. A single master device can be connected to more than one iommu and multiple contexts in each of the iommu. This association between master and iommus was represented from platform data using parent/child devices. The master drivers were responsible for attaching all of the iommus/context to a domain. Now the platform data support is removed and DT support is added. The master/iommus are added through generic iommu bindings.
This is essentially rework of the patch posted earlier by Rob Clark <[email protected]>. This series folds the changes in to the existing driver with the addition of generic bindings.
http://www.spinics.net/lists/linux-arm-msm/msg10077.html
This series is based on the IOMMU probe deferral series from Laurent Pinchart <[email protected]>.
https://lkml.org/lkml/2015/5/14/786
Tested this series on ifc6410 board.
[V2] Adapted the driver to use generic ARMV7S short descriptor pagetable ops
and addressed comments.
[V1]
https://lists.linuxfoundation.org/pipermail/iommu/2015-August/014074.html
Sricharan R (5):
iommu/msm: Add DT adaptation
iommu/msm: Move the contents from msm_iommu_dev.c to msm_iommu.c iommu/msm: Add support for generic master bindings iommu/msm: use generic ARMV7S short descriptor pagetable ops iommu/msm: Remove driver BROKEN
.../devicetree/bindings/iommu/msm,iommu-v0.txt | 59 ++ drivers/iommu/Kconfig | 2 +- drivers/iommu/Makefile | 2 +- drivers/iommu/msm_iommu.c | 836 +++++++++++---------- drivers/iommu/msm_iommu.h | 73 +- drivers/iommu/msm_iommu_dev.c | 381 ---------- 6 files changed, 523 insertions(+), 830 deletions(-) create mode 100644 Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt delete mode 100644 drivers/iommu/msm_iommu_dev.c
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 1/5 iommu/msm: Add DT adaptation
The driver currently works based on platform data. Remove this and add support for DT. A single master can have multiple ports connected to more than one iommu.
master
|
|
|
------------------------
| |
IOMMU0 IOMMU1
| |
ctx0 ctx1 ctx0 ctx1This association of master and iommus/contexts were previously represented by platform data parent/child device details. The client drivers were responsible for programming all of the iommus/contexts for the device. Now while adapting to generic DT bindings we maintain the list of iommus, contexts that each master domain is connected to and program all of them on attach/detach.
--- .../devicetree/bindings/iommu/msm,iommu-v0.txt | 59 ++++ drivers/iommu/msm_iommu.c | 252 +++++++++-------- drivers/iommu/msm_iommu.h | 73 ++--- drivers/iommu/msm_iommu_dev.c | 315 +++++---------------- 4 files changed, 296 insertions(+), 403 deletions(-) create mode 100644 Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +59 -0
@@ -0,0 +1,59 @@ +* QCOM IOMMU + +The QCOM IOMMU is an implementation compatible with the ARM VMSA short +descriptor page tables. It provides address translation for bus masters outside +of the CPU, each connected to the IOMMU through a port called micro-TLB. + +Required Properties: + + - compatible: Must contain "qcom,iommu-v0". + - reg: Base address and size of the IOMMU registers. + - interrupts: Specifiers for the MMU fault interrupts. For instances that + support secure mode two interrupts must be specified, for non-secure and + secure mode, in that order. For instances that don't support secure mode a + single interrupt must be specified. + - #iommu-cells: This is the total number of stream ids that a master would + use during transactions which will be specified as a list + as a part of iommus property below. + - ncb: The total number of context banks in the IOMMU. + - clocks : List of clocks to be used during SMMU register access. See + Documentation/devicetree/bindings/clock/clock-bindings.txt + for information about the format. For each clock specified + here, there must be a corresponding entry in clock-names + (see below). + + - clock-names : List of clock names corresponding to the clocks specified in + the "clocks" property (above). See + Documentation/devicetree/bindings/clock/clock-bindings.txt + for more info. + +Each bus master connected to an IOMMU must reference the IOMMU in its device +node with the following property: + + - iommus: A reference to the IOMMU in multiple cells. The first cell is a + phandle to the IOMMU and the second cell is the list of the + stream ids used by the device. + +Example: mdp iommu and its bus master + + mdp_port0: qcom,iommu at 7500000 { + compatible = "msm,iommu-v0"; + #iommu-cells = <2>; + clock-names = + "smmu_pclk", + "iommu_clk"; + clocks = + <&mmcc SMMU_AHB_CLK>, + <&mmcc MDP_AXI_CLK>; + reg = <0x07500000 0x100000>; + interrupts = + <GIC_SPI 63 0>, + <GIC_SPI 64 0>; + ncb = <2>; + }; + + mdp: qcom,mdp at 5100000 { + compatible = "qcom,mdp"; + ... + iommus = <&mdp_port0 0 2>; + };
drivers/iommu/msm_iommu.c +138 -114
381 lines not on this page · raw message
drivers/iommu/msm_iommu.h +26 -47
100 lines not on this page · raw message
drivers/iommu/msm_iommu_dev.c +73 -242
370 lines not on this page · raw message
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 1/5 iommu/msm: Add DT adaptation
Hi Sricharan,
Thank you for the patch.
Quoted 56 lines · On Wednesday 06 Apr 2016 19:59:31 Sricharan R wrote:
The driver currently works based on platform data. Remove this and add support for DT. A single master can have multiple ports connected to more than one iommu.
master | | | ------------------------ | | IOMMU0 IOMMU1 | | ctx0 ctx1 ctx0 ctx1This association of master and iommus/contexts were previously represented by platform data parent/child device details. The client drivers were responsible for programming all of the iommus/contexts for the device. Now while adapting to generic DT bindings we maintain the list of iommus, contexts that each master domain is connected to and program all of them on attach/detach.
Signed-off-by Sricharan R <[email protected]>--- .../devicetree/bindings/iommu/msm,iommu-v0.txt | 59 ++++ drivers/iommu/msm_iommu.c | 252 +++++++++-------- drivers/iommu/msm_iommu.h | 73 ++--- drivers/iommu/msm_iommu_dev.c | 315 ++++-------------- 4 files changed, 296 insertions(+), 403 deletions(-) create mode 100644 Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt(quoted diff) +0
-0
b/Documentation/devicetree/bindings/iommu/msm,iommu-v0.txtnew file mode 100644
index 0000000..21bfbfcDocumentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +4
-0@@ -0,0 +1,59 @@ +* QCOM IOMMU + +The QCOM IOMMU is an implementation compatible with the ARM VMSA short +descriptor page tables. It provides address translation for bus mastersoutside
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +8
-0+of the CPU, each connected to the IOMMU through a port called micro-TLB. + +Required Properties: + + - compatible: Must contain "qcom,iommu-v0". + - reg: Base address and size of the IOMMU registers. + - interrupts: Specifiers for the MMU fault interrupts. For instances that + support secure mode two interrupts must be specified, for non-secureand
+ secure mode, in that order. For instances that don't support secure
mode aDocumentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +2
-0+ single interrupt must be specified. + - #iommu-cells: This is the total number of stream ids that a masterwould
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt line 17
Quoted 2 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +2
-0+ use during transactions which will be specified as a list + as a part of iommus property below.
That's not correct. #iommu-cells, as defined in the core IOMMU DT bindings, is "the number of cells in an IOMMU specifier needed to encode an address" (address being a stream id here).
Can the number of cells differ from instance to instance, or is it always 2 ?
Quoted 1 line
+ - ncb: The total number of context banks in the IOMMU.
Should this be qcom,ncb ?
Quoted 14 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +7
-0+ - clocks : List of clocks to be used during SMMU register access. See + Documentation/devicetree/bindings/clock/clock-bindings.txt + for information about the format. For each clock specified + here, there must be a corresponding entry in clock-names + (see below). + + - clock-names : List of clock names corresponding to the clocks specifiedin
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +5
-0+ the "clocks" property (above). See + Documentation/devicetree/bindings/clock/clock-bindings.txt + for more info. + +Each bus master connected to an IOMMU must reference the IOMMU in itsdevice
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt line 35
Quoted 5 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +5
-0+node with the following property: + + - iommus: A reference to the IOMMU in multiple cells. The first cell is a + phandle to the IOMMU and the second cell is the list of the + stream ids used by the device.
You mention in your cover letter that a master device can be connected to multiple iommus, shouldn't that be stated here ? On the same topic, do your masters need to selectively enable/disable memory ports to IOMMUs, or can they all be enabled/disabled together ?
Also, the second cell can't be a list of stream ids, as one cell stores one value. A master device using multiple stream ids should use multiple entries in the iommus property.
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt line 39
Quoted 3 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +3
-0+Example: mdp iommu and its bus master + + mdp_port0: qcom,iommu at 7500000 {
I think you can use iommu instead of qcom,iommu.
Quoted 20 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +20
-0+ compatible = "msm,iommu-v0"; + #iommu-cells = <2>; + clock-names = + "smmu_pclk", + "iommu_clk"; + clocks = + <&mmcc SMMU_AHB_CLK>, + <&mmcc MDP_AXI_CLK>; + reg = <0x07500000 0x100000>; + interrupts = + <GIC_SPI 63 0>, + <GIC_SPI 64 0>; + ncb = <2>; + }; + + mdp: qcom,mdp at 5100000 { + compatible = "qcom,mdp"; + ... + iommus = <&mdp_port0 0 2>; + };
Signature · 3 lines
Regards, Laurent Pinchart
PATCH v2 1/5 iommu/msm: Add DT adaptation
Hi Laurent,
Thanks for the review.
Quoted 7 lines
+ - #iommu-cells: This is the total number of stream ids that a
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt line 17
Quoted 4 lines
+ master
wouldDocumentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +2
-0+ use during transactions which will be specified as a list + as a part of iommus property below.That's not correct. #iommu-cells, as defined in the core IOMMU DT
bindings,
Quoted 4 lines
is "the number of cells in an IOMMU specifier needed to encode an address" (address being a stream id here).
Can the number of cells differ from instance to instance, or is it always
2 ?
Ah, you are right. For this and the below, this should be ideally '1'
as the
Stream ids for a master here are represented by a single cell. I should change my xlate function to take care of this.
Quoted 5 lines
Quoted 2 lines
+ - ncb: The total number of context banks in the IOMMU.
Should this be qcom,ncb ?
Ya, will change this.
Quoted 13 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ - clocks : List of clocks to be used during SMMU register access. See + Documentation/devicetree/bindings/clock/clock-bindings.txt
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt
Quoted 5 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +5
-0+ for information about the format. For each clock specified + here, there must be a corresponding entry in clock-names + (see below). + + - clock-names : List of clock names corresponding to the clocksspecified
Review
Quoted 3 lines
in
(quoted diff) +2
-0+ the "clocks" property (above). See + Documentation/devicetree/bindings/clock/clock-bindings.txt
Quoted 8 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt
Quoted 3 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +3
-0+ for more info. + +Each bus master connected to an IOMMU must reference the IOMMU inits
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt
Quoted 4 lines
device
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +3
-0+node with the following property: + + - iommus: A reference to the IOMMU in multiple cells. The first cell
is a
Quoted 5 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt line 35
Quoted 2 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +2
-0+ phandle to the IOMMU and the second cell is the list of the + stream ids used by the device.You mention in your cover letter that a master device can be connected to multiple iommus, shouldn't that be stated here ? On the same topic, do
your
Quoted 3 lines
masters need to selectively enable/disable memory ports to IOMMUs, or can they all be enabled/disabled together ?
They are connected to all iommus and ports and enabled/disabled together. I will add little more description here for this here.
Quoted 1 line
Also, the second cell can't be a list of stream ids, as one cell stores
one value.
Quoted 1 line
A master device using multiple stream ids should use multiple entries in
the
Quoted 2 lines
iommus property.
Yes, true. Wrong usage here and as I said above should be corrected.
Quoted 5 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt line 39
Quoted 3 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +3
-0+Example: mdp iommu and its bus master + + mdp_port0: qcom,iommu at 7500000 {I think you can use iommu instead of qcom,iommu.
Ok, right.
Regards,
Sricharan
Rob Herring
maintainerPATCH v2 1/5 iommu/msm: Add DT adaptation
In reply to Sricharan RQuoted 11 lines · Sricharan R
The driver currently works based on platform data. Remove this and add support for DT. A single master can have multiple ports connected to more than one iommu.
master | | | ------------------------ | | IOMMU0 IOMMU1
You've got mixtures of tabs and spaces here. Either fully use leading tabs or use spaces everywhere.
Quoted 10 lines
| | ctx0 ctx1 ctx0 ctx1This association of master and iommus/contexts were previously represented by platform data parent/child device details. The client drivers were responsible for programming all of the iommus/contexts for the device. Now while adapting to generic DT bindings we maintain the list of iommus, contexts that each master domain is connected to and program all of them on attach/detach.
Quoted 3 lines
Signed-off-by Sricharan R <[email protected]>--- .../devicetree/bindings/iommu/msm,iommu-v0.txt | 59 ++++
It is preferred to split bindings to separate patch.
Quoted 12 lines
drivers/iommu/msm_iommu.c | 252 +++++++++-------- drivers/iommu/msm_iommu.h | 73 ++--- drivers/iommu/msm_iommu_dev.c | 315 +++++---------------- 4 files changed, 296 insertions(+), 403 deletions(-) create mode 100644 Documentation/devicetree/bindings/iommu/msm,iommu-v0.txtDocumentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +0
-0
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt line 9
Quoted 9 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +9
-0@@ -0,0 +1,59 @@ +* QCOM IOMMU + +The QCOM IOMMU is an implementation compatible with the ARM VMSA short +descriptor page tables. It provides address translation for bus masters outside +of the CPU, each connected to the IOMMU through a port called micro-TLB. + +Required Properties: + + - compatible: Must contain "qcom,iommu-v0".
You need SoC specific compatible strings in addition.
Quoted 50 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +50
-0+ - reg: Base address and size of the IOMMU registers. + - interrupts: Specifiers for the MMU fault interrupts. For instances that + support secure mode two interrupts must be specified, for non-secure and + secure mode, in that order. For instances that don't support secure mode a + single interrupt must be specified. + - #iommu-cells: This is the total number of stream ids that a master would + use during transactions which will be specified as a list + as a part of iommus property below. + - ncb: The total number of context banks in the IOMMU. + - clocks : List of clocks to be used during SMMU register access. See + Documentation/devicetree/bindings/clock/clock-bindings.txt + for information about the format. For each clock specified + here, there must be a corresponding entry in clock-names + (see below). + + - clock-names : List of clock names corresponding to the clocks specified in + the "clocks" property (above). See + Documentation/devicetree/bindings/clock/clock-bindings.txt + for more info. + +Each bus master connected to an IOMMU must reference the IOMMU in its device +node with the following property: + + - iommus: A reference to the IOMMU in multiple cells. The first cell is a + phandle to the IOMMU and the second cell is the list of the + stream ids used by the device. + +Example: mdp iommu and its bus master + + mdp_port0: qcom,iommu at 7500000 { + compatible = "msm,iommu-v0"; + #iommu-cells = <2>; + clock-names = + "smmu_pclk", + "iommu_clk"; + clocks = + <&mmcc SMMU_AHB_CLK>, + <&mmcc MDP_AXI_CLK>; + reg = <0x07500000 0x100000>; + interrupts = + <GIC_SPI 63 0>, + <GIC_SPI 64 0>; + ncb = <2>; + }; + + mdp: qcom,mdp at 5100000 { + compatible = "qcom,mdp"; + ... + iommus = <&mdp_port0 0 2>; + };
PATCH v2 1/5 iommu/msm: Add DT adaptation
Hi Rob,
Quoted 13 lines · Sricharan R
Quoted 11 lines
The driver currently works based on platform data. Remove this and add support for DT. A single master can have multiple ports connected to more than one iommu.
master | | | ------------------------ | | IOMMU0 IOMMU1
Quoted 1 line
You've got mixtures of tabs and spaces here. Either fully use leading tabs
or
Quoted 1 line
use spaces everywhere.
Ok, will change this properly.
Quoted 6 lines
Quoted 4 lines
| | ctx0 ctx1 ctx0 ctx1It is preferred to split bindings to separate patch.
Ok, will split it.
Quoted 11 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+The QCOM IOMMU is an implementation compatible with the ARM VMSA +short descriptor page tables. It provides address translation for bus +masters outside of the CPU, each connected to the IOMMU through aport called micro-TLB.
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt line 9
Quoted 4 lines
Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt new file +4
-0+ +Required Properties: + + - compatible: Must contain "qcom,iommu-v0".You need SoC specific compatible strings in addition.
Ok, will add it.
Regards,
Sricharan
PATCH v2 2/5 iommu/msm: Move the contents from msm_iommu_dev.c to msm_iommu.c
In reply to Sricharan RThere are only two functions left in msm_iommu_dev.c. Move it to msm_iommu.c and delete the file.
--- drivers/iommu/Makefile | 2 +- drivers/iommu/msm_iommu.c | 182 ++++++++++++++++++++++++++++++++++++ drivers/iommu/msm_iommu_dev.c | 212 ------------------------------------------ 3 files changed, 183 insertions(+), 213 deletions(-) delete mode 100644 drivers/iommu/msm_iommu_dev.c
drivers/iommu/Makefile +1 -1
@@ -7,7 +7,7 @@ obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o obj-$(CONFIG_IOMMU_IOVA) += iova.o obj-$(CONFIG_OF_IOMMU) += of_iommu.o -obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o msm_iommu_dev.o +obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o obj-$(CONFIG_QCOM_IOMMU_V1) += qcom/ obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
drivers/iommu/msm_iommu.c +182 -0
200 lines not on this page · raw message
drivers/iommu/msm_iommu_dev.c deleted +0 -212
212 lines not on this page · raw message
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 3/5 iommu/msm: Add support for generic master bindings
In reply to Sricharan RThis adds the xlate callback which gets invoked during device registration from DT. The master devices gets added through this.
--- drivers/iommu/msm_iommu.c | 54 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-)
drivers/iommu/msm_iommu.c +52 -2
@@ -28,6 +28,7 @@#include <linux/iommu.h> #include <linux/clk.h> #include <linux/err.h> +#include <linux/of_iommu.h> #include <asm/cacheflush.h> #include <asm/sizes.h>
@@ -702,6 +703,47 @@ static void print_ctx_regs(void __iomem *base, int ctx) GET_PRRR(base, ctx), GET_NMRR(base, ctx)); } +static void insert_iommu_master (struct device *dev, +struct msm_iommu_dev *iommu, +struct of_phandle_args *spec) +{ +struct msm_iommu_ctx_dev *master; +int sid; + + master =kzalloc (sizeof (*master),GFP_ATOMIC ); + master->of_node = dev->of_node; +list_add (&master->list, &iommu->ctx_list); + +for (sid =0 ; sid < spec->args_count; sid++) + master->mids[sid] = spec->args[sid]; + + master->num_mids = spec->args_count; +} + +static int qcom_iommu_of_xlate (struct device *dev, +struct of_phandle_args *spec) +{ +struct msm_iommu_dev *iommu; +unsigned long flags; +int ret =0 ; + +spin_lock_irqsave (&msm_iommu_lock, flags); +list_for_each_entry (iommu, &qcom_iommu_devices, dev_node) +if (iommu->dev->of_node == spec->np) +break ; + +if (!iommu || iommu->dev->of_node != spec->np) { + ret = -ENODEV ; + goto fail; + } + +insert_iommu_master (dev, iommu, spec); +fail: +spin_unlock_irqrestore (&msm_iommu_lock, flags); + +return ret; +} +irqreturn_t msm_iommu_fault_handler (int irq,void *dev_id) {struct msm_iommu_dev *iommu = dev_id;
@@ -737,7 +779,7 @@ fail:return 0 ; } -static const struct iommu_ops msm_iommu_ops = { +static struct iommu_ops msm_iommu_ops = { .capable = msm_iommu_capable, .domain_alloc = msm_iommu_domain_alloc, .domain_free = msm_iommu_domain_free,
@@ -748,6 +790,7 @@ static const struct iommu_ops msm_iommu_ops = { .map_sg = default_iommu_map_sg, .iova_to_phys = msm_iommu_iova_to_phys, .pgsize_bitmap = MSM_IOMMU_PGSIZES, + .of_xlate = qcom_iommu_of_xlate, };static int msm_iommu_probe (struct platform_device *pdev)
@@ -837,6 +880,7 @@ static int msm_iommu_probe(struct platform_device *pdev) }list_add (&iommu->dev_node, &qcom_iommu_devices); +of_iommu_set_ops (pdev->dev.of_node, &msm_iommu_ops);pr_info ("device mapped at %p, irq %d with %d ctx banks\n" , iommu->base, iommu->irq, iommu->ncb);
@@ -935,7 +979,13 @@ static int __init msm_iommu_init(void)return 0 ; } -subsys_initcall (msm_iommu_init); +static int __init msm_iommu_of_setup (struct device_node *np) +{ +msm_iommu_init (); +return 0 ; +} + +IOMMU_OF_DECLARE (msm_iommu_of,"qcom,iommu-v0" , msm_iommu_of_setup);MODULE_LICENSE ("GPL v2" );MODULE_AUTHOR ("Stepan Moskovchenko <[email protected]>" );
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 4/5 iommu/msm: use generic ARMV7S short descriptor pagetable ops
In reply to Sricharan RThis iommu uses the armv7 short descriptor format. So use the generic ARMV7S pagetable ops instead of rewriting the same stuff in the driver.
--- drivers/iommu/Kconfig | 1 + drivers/iommu/msm_iommu.c | 378 ++++++++++------------------------------------ 2 files changed, 80 insertions(+), 299 deletions(-)
drivers/iommu/Kconfig +1 -0
@@ -92,6 +92,7 @@ config MSM_IOMMU depends on ARCH_MSM8X60 || ARCH_MSM8960 || COMPILE_TEST depends on BROKEN select IOMMU_API + select IOMMU_IO_PGTABLE_ARMV7S help Support for the IOMMUs found on certain Qualcomm SOCs. These IOMMUs allow virtualization of the address space used by most
drivers/iommu/msm_iommu.c +79 -299
524 lines not on this page · raw message
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 4/5 iommu/msm: use generic ARMV7S short descriptor pagetable ops
Quoted 9 lines · Sricharan R
This iommu uses the armv7 short descriptor format. So use the generic ARMV7S pagetable ops instead of rewriting the same stuff in the driver.
Signed-off-by Sricharan R <[email protected]>--- drivers/iommu/Kconfig | 1 + drivers/iommu/msm_iommu.c | 378 ++++++++++------------------------------------ 2 files changed, 80 insertions(+), 299 deletions(-)
Nice diffstat!
Quoted 29 lines
drivers/iommu/msm_iommu.c +2
-17+static void __flush_iotlb (void *cookie) { -struct msm_priv *priv =to_msm_priv (domain); +struct msm_priv *priv = cookie;struct msm_iommu_dev *iommu =NULL ;struct msm_iommu_ctx_dev *master;int ret =0 ; -#ifndef CONFIG_IOMMU_PGTABLES_L2 -unsigned long *fl_table = priv->pgtable; -int i; - -if (!list_empty (&priv->list_attached)) { -dmac_flush_range (fl_table, fl_table +SZ_16K ); - -for (i =0 ; i <NUM_FL_PTE ; i++) -if ((fl_table[i] &0x03 ) ==FL_TYPE_TABLE ) { -void *sl_table =__va (fl_table[i] & -FL_BASE_MASK ); -dmac_flush_range (sl_table, sl_table +SZ_4K ); - } - } -#endif -list_for_each_entry (iommu, &priv->list_attached, dom_node) { ret =__enable_clocks (iommu);if (ret)@@ -162,9 +144,26 @@ static int __flush_iotlb(struct iommu_domain *domain)__disable_clocks (iommu);
drivers/iommu/msm_iommu.c static int __flush_iotlb(struct iommu_domain *domain) · line 153
Quoted 10 lines
drivers/iommu/msm_iommu.c +7
-1} fail: -return ret; +return ; +} + +static void __flush_iotlb_range (unsigned long iova,size_t size, +size_t granule,bool leaf,void *cookie) +{ +__flush_iotlb (cookie);
Can you really not do better than this? __flush_iotlb does a TLBIALL afaict, whereas you have the address range here and drivers/iommu/msm_iommu_hw-8xxx.h has entries for things like TLBIVA[A]...
You might even be able to make SET_CTX_REG use writel_relaxed and stick a DSB in the sync callback (assuming that's sufficient to complete the maintenance on your device).
Will
PATCH v2 4/5 iommu/msm: use generic ARMV7S short descriptor pagetable ops
Hi will,
Quoted 5 lines
-----Original Message-----
From: Will Deacon [mailto:will.deacon at arm.com]
Sent: Tuesday, April 26, 2016 7:47 PM
To: Sricharan R <[email protected]>
Cc: linux-arm-kernel at lists.infradead.org;
iommu at lists.linux-foundation.org;
Quoted 52 lines
devicetree at vger.kernel.org; linux-arm-msm at vger.kernel.org; robin.murphy at arm.com; robdclark at gmail.com; joro at 8bytes.org; srinivas.kandagatla at linaro.org; laurent.pinchart at ideasonboard.com; stepanm at codeaurora.org; treding at nvidia.com
Subject: Re: [PATCH V2 4/5] iommu/msm: use generic ARMV7S short descriptor pagetable opsQuoted 10 lines · Sricharan R
This iommu uses the armv7 short descriptor format. So use the generic ARMV7S pagetable ops instead of rewriting the same stuff in the driver.
Signed-off-by Sricharan R <[email protected]>--- drivers/iommu/Kconfig | 1 + drivers/iommu/msm_iommu.c | 378 ++++++++++------------------------------------ 2 files changed, 80 insertions(+), 299 deletions(-)Nice diffstat!
Quoted 9 lines
drivers/iommu/msm_iommu.c +2
-2+static void __flush_iotlb (void *cookie) { -struct msm_priv *priv =to_msm_priv (domain); +struct msm_priv *priv = cookie;struct msm_iommu_dev *iommu =NULL ;struct msm_iommu_ctx_dev *master;int ret =0 ; -#ifndef CONFIG_IOMMU_PGTABLES_L2 drivers/iommu/msm_iommu.c static void msm_iommu_reset(void __iomem *base, int ncb) · line 153
Quoted 10 lines
drivers/iommu/msm_iommu.c +0
-10-unsigned long *fl_table = priv->pgtable; -int i; - -if (!list_empty (&priv->list_attached)) { -dmac_flush_range (fl_table, fl_table +SZ_16K ); - -for (i =0 ; i <NUM_FL_PTE ; i++) -if ((fl_table[i] &0x03 ) ==FL_TYPE_TABLE ) { - void *sl_table = __va(fl_table[i] & -FL_BASE_MASK);Review
Quoted 1 line
(quoted diff) +0
-1- dmac_flush_range(sl_table, sl_table +
SZ_4K);drivers/iommu/msm_iommu.c static int __flush_iotlb(struct iommu_domain
Quoted 8 lines
drivers/iommu/msm_iommu.c +0
-4- } - } -#endif -list_for_each_entry (iommu, &priv->list_attached, dom_node) { ret =__enable_clocks (iommu);if (ret)@@ -162,9 +144,26 @@ static int __flush_iotlb(struct iommu_domain*domain)
Quoted 12 lines
drivers/iommu/msm_iommu.c static int __flush_iotlb(struct iommu_domain *domain) · line 153
Quoted 10 lines
drivers/iommu/msm_iommu.c +6
-1__disable_clocks (iommu); } fail: -return ret; +return ; +} + +static void __flush_iotlb_range (unsigned long iova,size_t size, +size_t granule,bool leaf,void *cookie) { +__flush_iotlb (cookie);Can you really not do better than this?
__flush_iotlbdoes a TLBIALL
afaict,
Quoted 8 lines
whereas you have the address range here and
drivers/iommu/msm_iommu_hw-8xxx.hhas entries for things like TLBIVA[A]...You might even be able to make
SET_CTX_REGusewritel_relaxedand stick a DSB in the sync callback (assuming that's sufficient to complete the maintenance on your device).
Yes, this is a overkill. I should change this as you have mentioned.
Thanks.
Regards,
Sricharan
PATCH v2 5/5 iommu/msm: Remove driver BROKEN
In reply to Sricharan RNow that the driver is DT adapted, bus_set_iommu gets called only when on compatible matching. So the driver should not break multiplatform builds now. So remove the BROKEN config.
--- drivers/iommu/Kconfig | 1 - 1 file changed, 1 deletion(-)
drivers/iommu/Kconfig +0 -1
@@ -90,7 +90,6 @@ config MSM_IOMMU bool "MSM IOMMU Support" depends on ARM depends on ARCH_MSM8X60 || ARCH_MSM8960 || COMPILE_TEST - depends on BROKEN select IOMMU_API select IOMMU_IO_PGTABLE_ARMV7S help
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation