Re: [PATCH] pcie-rcar: try setting PCIe speed to 5 GT/s at boot

[Sergei Shtylyov]

Hello.

On 09/14/2016 11:54 PM, Bjorn Helgaas wrote:


From: Grigory Kletsko 

Initially, the PCIe link speed is set up only at 2.5 GT/s.
For better performance,  we're trying to increase link speed to 5 GT/s.

[Sergei Shtylyov: indented the macro definitions with tabs, renamed the
SPCHG register bits for consistency, renamed the link speed field/values,
fixed too long lines, fixed redundancy in clearing the MACSR register bits,
fixed grammar/typos in  the comments/messages, removed unrelated/useless
changes, fixed bugs in rcar_rwm32() calls done to set the bits, removed
unneeded braces, removed non-informative comment, reworded the patch
summary/description.]

Signed-off-by: Grigory Kletsko 
Signed-off-by: Sergei Shtylyov 

---
The patch is against the 'next' branch of Bjorn Helgaas' 'pci.git' repo.

 drivers/pci/host/pcie-rcar.c |  103 +++
 1 file changed, 103 insertions(+)

Index: pci/drivers/pci/host/pcie-rcar.c
===
--- pci.orig/drivers/pci/host/pcie-rcar.c
+++ pci/drivers/pci/host/pcie-rcar.c

[...]

@@ -385,6 +402,51 @@ static int rcar_pcie_setup(struct list_h
return 1;
 }

+void rcar_pcie_force_speedup(struct rcar_pcie *pcie)
+{
+   u32 macsr;
+
+   dev_info(pcie->dev, "Trying speed up to 5 GT/s\n");
+
+   if ((rcar_pci_read_reg(pcie, MACSR) & SPCHGFIN) ||
+   (rcar_pci_read_reg(pcie, MACCTLR) & SPEED_CHANGE)) {
+   dev_err(pcie->dev, "Speed changing is in progress\n");


Are these messages all really errors?


I guess not. :-)


+   return;
+   }
+
+   if ((rcar_pci_read_reg(pcie, MACSR) & LINK_SPEED) ==
+   LINK_SPEED_5_0GTS) {
+   dev_err(pcie->dev, "Current link speed is already 5 GT/s\n");
+   return;
+   }
+
+   if ((rcar_pci_read_reg(pcie, MACS2R) & LINK_SPEED) !=
+   LINK_SPEED_5_0GTS) {
+   dev_err(pcie->dev,
+   "Current max support link speed not 5 GT/s\n");
+   return;
+   }

[...]

@@ -416,6 +478,9 @@ static int rcar_pcie_enable(struct rcar_

pci_bus_add_devices(bus);

+   /* Try setting 5 GT/s link speed */
+   rcar_pcie_force_speedup(pcie);


I assume it's safe to change the link speed even while the downstream
devices are in use?  As soon as we call pci_bus_add_devices(), drivers
can claim the devices and start using them.


   Not sure. OK, I'm going to move this call before pci_bus_add_devices() and 
try to make it synchronous, without using interrupt.



return 0;
 }

@@ -621,6 +686,44 @@ static irqreturn_t rcar_pcie_msi_irq(int
struct rcar_msi *msi = >msi;
unsigned long reg;

+   if (rcar_pci_read_reg(pcie, PCIEINTR) & PCIEINTMAC) {
+   dev_dbg(pcie->dev, "MAC interrupt received\n");


I guess you don't need to write PCIEINTR or any other register to
acknowledge the interrupt?


   You need to write MACSR -- which the code below does. However, it only 
clears the SPCHGFIN interrupt (which is only ever enabled).



+
+   rcar_rmw32(pcie, MACSR, SPCHGFIN, SPCHGFIN);
+
+   /* Disable this interrupt */
+   rcar_rmw32(pcie, MACINTENR, SPCHGFINE, 0);
+   rcar_rmw32(pcie, PCIEINTER, PCIEINTMACE, 0);



reg = rcar_pci_read_reg(pcie, PCIEMSIFR);

/* MSI & INTx share an interrupt - we only handle MSI here */


Is this patch adding handling for INTx events?


No.

MBR, Sergei

[PATCH v2 1/3] ARM: dts: r7s72100: add mmcif to device tree

[Chris Brandt]

Signed-off-by: Chris Brandt 
---
v2:
* added card detect as 3rd interupt source
---
 arch/arm/boot/dts/r7s72100.dtsi | 12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index e18d4e6..50f9f3b 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -450,4 +450,16 @@
#size-cells = <0>;
status = "disabled";
};
+
+   mmcif: mmc@e804c800 {
+   compatible = "renesas,mmcif-r7s72100", "renesas,sh-mmcif";
+   reg = <0xe804c800 0x80>;
+   interrupts = ;
+   clocks = <_clks R7S72100_CLK_MMCIF>;
+   reg-io-width = <4>;
+   bus-width = <8>;
+   status = "disabled";
+   };
 };
-- 
2.9.2

[PATCH v2 2/3] mmc: sh_mmcif: Document r7s72100 DT bindings

[Chris Brandt]

Signed-off-by: Chris Brandt 
---
v2:
* added interrupt description
---
 Documentation/devicetree/bindings/mmc/renesas,mmcif.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt 
b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
index ff611fa..1443edb 100644
--- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
+++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
@@ -8,12 +8,16 @@ Required properties:
 
 - compatible: should be "renesas,mmcif-", "renesas,sh-mmcif" as a
   fallback. Examples with  are:
+   - "renesas,mmcif-r7s72100" for the MMCIF found in r7s72100 SoCs
- "renesas,mmcif-r8a7740" for the MMCIF found in r8a7740 SoCs
- "renesas,mmcif-r8a7790" for the MMCIF found in r8a7790 SoCs
- "renesas,mmcif-r8a7791" for the MMCIF found in r8a7791 SoCs
- "renesas,mmcif-r8a7793" for the MMCIF found in r8a7793 SoCs
- "renesas,mmcif-r8a7794" for the MMCIF found in r8a7794 SoCs
 
+- interrupts: either 1 shared interrupt, 2 separate interrupts (error and int),
+  or 3 separate interrupts (error, int, card detect)
+
 - clocks: reference to the functional clock
 
 - dmas: reference to the DMA channels, one per channel name listed in the
-- 
2.9.2

[PATCH v2 3/3] ARM: dts: rskrza1: add mmc DT support

[Chris Brandt]

Since the MMC and SDHI1 on the RSK share the same socket connector (CN1),
you cannot enable MMC and SDHI1 at the same time. Therefore the status
has been set to disabled because SDHI is more popular with this board.
However, keeping this code in here serves as a good way to document how
the MMC on the RZ/A1 has been known to work for someone that does want
to use MMC instead of SDHI1.

A fixed 3.3 regulator is included because it is required by the mmc
driver.

Signed-off-by: Chris Brandt 
---
 arch/arm/boot/dts/r7s72100-rskrza1.dts | 17 +
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100-rskrza1.dts 
b/arch/arm/boot/dts/r7s72100-rskrza1.dts
index e5dea5b..9228b04 100644
--- a/arch/arm/boot/dts/r7s72100-rskrza1.dts
+++ b/arch/arm/boot/dts/r7s72100-rskrza1.dts
@@ -33,6 +33,15 @@
#address-cells = <1>;
#size-cells = <1>;
};
+
+   d3_3v: regulator-d3-3v {
+   compatible = "regulator-fixed";
+   regulator-name = "D3.3V";
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   regulator-boot-on;
+   regulator-always-on;
+   };
 };
 
 _clk {
@@ -56,6 +65,14 @@
};
 };
 
+ {
+   vmmc-supply = <_3v>;
+   vqmmc-supply = <_3v>;
+   bus-width = <8>;
+   non-removable;
+   status = "disabled"; /* shares CN1 with sdhi1 */
+};
+
  {
status = "okay";
 };
-- 
2.9.2

[PATCH v2 0/3] r7s72100: dts: enable mmcif

[Chris Brandt]

This series enables the mmc driver for the RZ/A1.
Nothing needed to be changed with the actual sh_mmcif driver.
It worked fine as-is.

As you can see, the status in the rskrza1 dst was left
as disabled because I wanted to leave that code in there
for refernce for someone later that actually has a board with
an actaul eMMC chip. I tested with an MMC Plus card.

For version 2:
* First first patch that adds the clock has already been accepted
* Add in the 3rd "card detect" interrupt that no one uses, but it
  is documented in the hardware manual so it is reflected in the
  DT.

Chris Brandt (3):
  ARM: dts: r7s72100: add mmcif to device tree
  mmc: sh_mmcif: Document r7s72100 DT bindings
  ARM: dts: rskrza1: add mmc DT support

 Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |  4 
 arch/arm/boot/dts/r7s72100-rskrza1.dts  | 17 +
 arch/arm/boot/dts/r7s72100.dtsi | 12 
 3 files changed, 33 insertions(+)

-- 
2.9.2

Re: [PATCH/RFC] iommu/ipmmu-vmsa: Update ->add_device() return value

[Magnus Damm]

Hi Robin,

Thanks for your feedback!!

On Tue, Sep 20, 2016 at 10:18 PM, Robin Murphy  wrote:
> Hi Magnus,
>
> On 20/09/16 13:41, Magnus Damm wrote:
>> From: Magnus Damm 
>>
>> Update the IPMMU driver to return -ENODEV when adding devices
>> not hooked up a particular IPMMU instance.
>>
>> Currently the ->add_device() callback implementation in the IPMMU
>> driver returns -ENODEV for devices with no "iommus" property,
>> however the function ipmmu_find_utlbs() may return -EINVAL.
>
> If there were no "iommus" property at all, of_parse_phandle_with_args()
> should return -ENOENT - that probably does want to be caught and passed
> back as -ENODEV to imply an untranslated device. On the other hand,
> -EINVAL would stem from the existence of the property, but in a somehow
> erroneous manner - other than the "args.np != mmu->dev->of_node" check
> (which could legitimately fail and be safely ignored if there are
> multiple IOMMUs in the system), any other reason implies a DT error
> which probably shouldn't be papered over.

Regarding -ENOENT to -ENODEV, I agree but I believe this case is
already handled. The ->add_device() callback seems to be using
of_count_phandle_with_args() to check for the presence of "iommus"
property before calling ipmmu_find_utlbs(). So for the case with no
"iommus" property at all it is returned as -ENODEV as you suggest.

The ->add_device() callback has the ret variable initialised to
-ENODEV by default. In case there is only one IPMMU device on the
ipmmu_device list and it matches the struct device passed to the
ipmmu_add_device() function then all is fine. However when there are
more than one IPMMU device on the ipmmu_device list then
ipmmu_find_utlbs() may return -EINVAL. Judging by the code this seems
to happen when the order of the IPMMU devices on the "iommus" property
does not match the IPMMU order on the ipmmu_device list.

Hm, I wonder if all this should be replaced with ->xlate() somehow?

Thanks,

/ magnus

Re: [PATCH/RFC v2 1/7] spi: Document DT bindings for SPI controllers in slave mode

[Rob Herring]

On Mon, Sep 12, 2016 at 10:50:40PM +0200, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven 
> ---
> v2:
>   - Do not create a child node in SPI slave mode. Instead, add an
> "spi-slave" property, and put the mode properties in the controller
> node.
> ---
>  Documentation/devicetree/bindings/spi/spi-bus.txt | 34 
> ++-
>  1 file changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt 
> b/Documentation/devicetree/bindings/spi/spi-bus.txt
> index 17822860cb98c34d..1ae28d7cafb68dc5 100644
> --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
> @@ -1,17 +1,23 @@
>  SPI (Serial Peripheral Interface) busses
>  
> -SPI busses can be described with a node for the SPI master device
> -and a set of child nodes for each SPI slave on the bus.  For this
> -discussion, it is assumed that the system's SPI controller is in
> -SPI master mode.  This binding does not describe SPI controllers
> -in slave mode.
> +SPI busses can be described with a node for the SPI controller device
> +and a set of child nodes for each SPI slave on the bus.  The system's SPI
> +controller may be described for use in SPI master mode or in SPI slave mode,
> +but not for both at the same time.
>  
> -The SPI master node requires the following properties:
> +The SPI controller node requires the following properties:
> +- compatible  - name of SPI bus controller following generic names
> + recommended practice.

We'll probably need some way to define what interface/protocol 
the slave has. Perhaps the most specific compatible should be the 
protocol the slave uses? Maybe that is how you use a child node?

> +
> +In master mode, the SPI controller node requires the following additional
> +properties:
>  - #address-cells  - number of cells required to define a chip select
>   address on the SPI bus.
>  - #size-cells - should be zero.
> -- compatible  - name of SPI bus controller following generic names
> - recommended practice.
> +
> +In slave mode, the SPI controller node requires one additional property:
> +- spi-slave   - Empty property.
> +
>  No other properties are required in the SPI bus node.  It is assumed
>  that a driver for an SPI bus device will understand that it is an SPI bus.
>  However, the binding does not attempt to define the specific method for
> @@ -21,7 +27,7 @@ assumption that board specific platform code will be used 
> to manage
>  chip selects.  Individual drivers can define additional properties to
>  support describing the chip select layout.
>  
> -Optional properties:
> +Optional properties (master mode only):
>  - cs-gpios - gpios chip select.
>  - num-cs   - total number of chipselects.
>  
> @@ -41,12 +47,14 @@ cs1 : native
>  cs2 :  1 0
>  cs3 :  2 0
>  
> -SPI slave nodes must be children of the SPI master node and can
> -contain the following properties.
> -- reg - (required) chip select address of device.
> +In master mode, SPI slave nodes must be children of the SPI controller node.
> +In slave mode, the (single) slave device is represented by the controller 
> node
> +itself. SPI slave nodes can contain the following properties.

I find this a bit confusing as you talk about master mode, then slave 
mode, then slave nodes (master mode again).

> +- reg - (required, master mode only) chip select address of 
> device.
>  - compatible  - (required) name of SPI device following generic names
>   recommended practice.
> -- spi-max-frequency - (required) Maximum SPI clocking speed of device in Hz.
> +- spi-max-frequency - (required, master mode only) Maximum SPI clocking speed
> + of device in Hz.
>  - spi-cpol- (optional) Empty property indicating device requires
>   inverse clock polarity (CPOL) mode.
>  - spi-cpha- (optional) Empty property indicating device requires
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 01/07] iommu/ipmmu-vmsa: Remove platform data handling

[Magnus Damm]

From: Magnus Damm 

The IPMMU driver is using DT these days, and platform data is no longer
used by the driver. Remove unused code.

Signed-off-by: Magnus Damm 
Reviewed-by: Laurent Pinchart 
---

 Changes since V4:
 - None

 Changes since V3:
 - None

 Changes since V2:
 - None

 Changes since V1:
 - Added Reviewed-by from Laurent

 drivers/iommu/ipmmu-vmsa.c |5 -
 1 file changed, 5 deletions(-)

--- 0002/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-09-20 21:46:26.0 +0900
@@ -766,11 +766,6 @@ static int ipmmu_probe(struct platform_d
int irq;
int ret;
 
-   if (!IS_ENABLED(CONFIG_OF) && !pdev->dev.platform_data) {
-   dev_err(>dev, "missing platform data\n");
-   return -EINVAL;
-   }
-
mmu = devm_kzalloc(>dev, sizeof(*mmu), GFP_KERNEL);
if (!mmu) {
dev_err(>dev, "cannot allocate device data\n");

[PATCH v5 04/07] iommu/ipmmu-vmsa: Break out domain allocation code

[Magnus Damm]

From: Magnus Damm 

Break out the domain allocation code into a separate function.
This is preparation for future code sharing.

Signed-off-by: Magnus Damm 
---

 Changes since V4:
 - None

 Changes since V3:
 - None

 Changes since V2:
 - Included this new patch as-is from the following series:
   [PATCH 00/04] iommu/ipmmu-vmsa: IPMMU CONFIG_IOMMU_DMA update

 drivers/iommu/ipmmu-vmsa.c |   13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

--- 0008/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-09-20 21:56:59.220607110 +0900
@@ -507,13 +507,10 @@ static irqreturn_t ipmmu_irq(int irq, vo
  * IOMMU Operations
  */
 
-static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
+static struct iommu_domain *__ipmmu_domain_alloc(unsigned type)
 {
struct ipmmu_vmsa_domain *domain;
 
-   if (type != IOMMU_DOMAIN_UNMANAGED)
-   return NULL;
-
domain = kzalloc(sizeof(*domain), GFP_KERNEL);
if (!domain)
return NULL;
@@ -523,6 +520,14 @@ static struct iommu_domain *ipmmu_domain
return >io_domain;
 }
 
+static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
+{
+   if (type != IOMMU_DOMAIN_UNMANAGED)
+   return NULL;
+
+   return __ipmmu_domain_alloc(type);
+}
+
 static void ipmmu_domain_free(struct iommu_domain *io_domain)
 {
struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);

[PATCH v5 03/07] iommu/ipmmu-vmsa: Break out utlb parsing code

[Magnus Damm]

From: Magnus Damm 

Break out the utlb parsing code and dev_data allocation into a
separate function. This is preparation for future code sharing.

Signed-off-by: Magnus Damm 
---

 Changes since V4:
 - Dropped hunk with fix to apply on top of:
   b1e2afc iommu/ipmmu-vmsa: Fix wrong error handle of ipmmu_add_device
 
 Changes since V3:
 - Initialize "mmu" to NULL, check before accessing
 - Removed group parameter from ipmmu_init_platform_device()

 Changes since V2:
 - Included this new patch from the following series:
   [PATCH 00/04] iommu/ipmmu-vmsa: IPMMU CONFIG_IOMMU_DMA update
 - Reworked code to fit on top on previous two patches in current series.

 drivers/iommu/ipmmu-vmsa.c |   58 
 1 file changed, 37 insertions(+), 21 deletions(-)

--- 0006/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-09-20 21:49:34.580607110 +0900
@@ -647,22 +647,15 @@ static int ipmmu_find_utlbs(struct ipmmu
return 0;
 }
 
-static int ipmmu_add_device(struct device *dev)
+static int ipmmu_init_platform_device(struct device *dev)
 {
struct ipmmu_vmsa_archdata *archdata;
struct ipmmu_vmsa_device *mmu;
-   struct iommu_group *group = NULL;
unsigned int *utlbs;
unsigned int i;
int num_utlbs;
int ret = -ENODEV;
 
-   if (dev->archdata.iommu) {
-   dev_warn(dev, "IOMMU driver already assigned to device %s\n",
-dev_name(dev));
-   return -EINVAL;
-   }
-
/* Find the master corresponding to the device. */
 
num_utlbs = of_count_phandle_with_args(dev->of_node, "iommus",
@@ -699,6 +692,36 @@ static int ipmmu_add_device(struct devic
}
}
 
+   archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
+   if (!archdata) {
+   ret = -ENOMEM;
+   goto error;
+   }
+
+   archdata->mmu = mmu;
+   archdata->utlbs = utlbs;
+   archdata->num_utlbs = num_utlbs;
+   dev->archdata.iommu = archdata;
+   return 0;
+
+error:
+   kfree(utlbs);
+   return ret;
+}
+
+static int ipmmu_add_device(struct device *dev)
+{
+   struct ipmmu_vmsa_archdata *archdata;
+   struct ipmmu_vmsa_device *mmu = NULL;
+   struct iommu_group *group;
+   int ret;
+
+   if (dev->archdata.iommu) {
+   dev_warn(dev, "IOMMU driver already assigned to device %s\n",
+dev_name(dev));
+   return -EINVAL;
+   }
+
/* Create a device group and add the device to it. */
group = iommu_group_alloc();
if (IS_ERR(group)) {
@@ -716,16 +739,9 @@ static int ipmmu_add_device(struct devic
goto error;
}
 
-   archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
-   if (!archdata) {
-   ret = -ENOMEM;
+   ret = ipmmu_init_platform_device(dev);
+   if (ret < 0)
goto error;
-   }
-
-   archdata->mmu = mmu;
-   archdata->utlbs = utlbs;
-   archdata->num_utlbs = num_utlbs;
-   dev->archdata.iommu = archdata;
 
/*
 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
@@ -736,6 +752,8 @@ static int ipmmu_add_device(struct devic
 * - Make the mapping size configurable ? We currently use a 2GB mapping
 *   at a 1GB offset to ensure that NULL VAs will fault.
 */
+   archdata = dev->archdata.iommu;
+   mmu = archdata->mmu;
if (!mmu->mapping) {
struct dma_iommu_mapping *mapping;
 
@@ -760,10 +778,8 @@ static int ipmmu_add_device(struct devic
return 0;
 
 error:
-   arm_iommu_release_mapping(mmu->mapping);
-
-   kfree(dev->archdata.iommu);
-   kfree(utlbs);
+   if (mmu)
+   arm_iommu_release_mapping(mmu->mapping);
 
dev->archdata.iommu = NULL;
 

[PATCH v5 02/07] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context

[Magnus Damm]

From: Magnus Damm 

Introduce a bitmap for context handing and convert the
interrupt routine to handle all registered contexts.

At this point the number of contexts are still limited.

Also remove the use of the ARM specific mapping variable
from ipmmu_irq() to allow compile on ARM64.

Signed-off-by: Magnus Damm 
---

 Changes since V4:
 - None

 Changes since V3:
 - None

 Changes since V2: (Thanks again to Laurent!)
 - Introduce a spinlock together with the bitmap and domain array.
 - Break out code into separate functions for alloc and free.
 - Perform free after (instead of before) configuring hardware registers.
 - Use the spinlock to protect the domain array in the interrupt handler.

 Changes since V1: (Thanks to Laurent for feedback!)
 - Use simple find_first_zero()/set_bit()/clear_bit() for context management.
 - For allocation rely on spinlock held when calling ipmmu_domain_init_context()
 - For test/free use atomic bitops
 - Return IRQ_HANDLED if any of the contexts generated interrupts

 drivers/iommu/ipmmu-vmsa.c |   76 ++--
 1 file changed, 66 insertions(+), 10 deletions(-)

--- 0004/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-09-20 21:48:23.770607110 +0900
@@ -8,6 +8,7 @@
  * the Free Software Foundation; version 2 of the License.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -26,12 +27,17 @@
 
 #include "io-pgtable.h"
 
+#define IPMMU_CTX_MAX 1
+
 struct ipmmu_vmsa_device {
struct device *dev;
void __iomem *base;
struct list_head list;
 
unsigned int num_utlbs;
+   spinlock_t lock;/* Protects ctx and domains[] */
+   DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
+   struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
 
struct dma_iommu_mapping *mapping;
 };
@@ -293,9 +299,29 @@ static struct iommu_gather_ops ipmmu_gat
  * Domain/Context Management
  */
 
+static int ipmmu_domain_allocate_context(struct ipmmu_vmsa_device *mmu,
+struct ipmmu_vmsa_domain *domain)
+{
+   unsigned long flags;
+   int ret;
+
+   spin_lock_irqsave(>lock, flags);
+
+   ret = find_first_zero_bit(mmu->ctx, IPMMU_CTX_MAX);
+   if (ret != IPMMU_CTX_MAX) {
+   mmu->domains[ret] = domain;
+   set_bit(ret, mmu->ctx);
+   }
+
+   spin_unlock_irqrestore(>lock, flags);
+
+   return ret;
+}
+
 static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
 {
u64 ttbr;
+   int ret;
 
/*
 * Allocate the page table operations.
@@ -325,10 +351,15 @@ static int ipmmu_domain_init_context(str
return -EINVAL;
 
/*
-* TODO: When adding support for multiple contexts, find an unused
-* context.
+* Find an unused context.
 */
-   domain->context_id = 0;
+   ret = ipmmu_domain_allocate_context(domain->mmu, domain);
+   if (ret == IPMMU_CTX_MAX) {
+   free_io_pgtable_ops(domain->iop);
+   return -EBUSY;
+   }
+
+   domain->context_id = ret;
 
/* TTBR0 */
ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
@@ -370,6 +401,19 @@ static int ipmmu_domain_init_context(str
return 0;
 }
 
+static void ipmmu_domain_free_context(struct ipmmu_vmsa_device *mmu,
+ unsigned int context_id)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(>lock, flags);
+
+   clear_bit(context_id, mmu->ctx);
+   mmu->domains[context_id] = NULL;
+
+   spin_unlock_irqrestore(>lock, flags);
+}
+
 static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
 {
/*
@@ -380,6 +424,7 @@ static void ipmmu_domain_destroy_context
 */
ipmmu_ctx_write(domain, IMCTR, IMCTR_FLUSH);
ipmmu_tlb_sync(domain);
+   ipmmu_domain_free_context(domain->mmu, domain->context_id);
 }
 
 /* 
-
@@ -437,16 +482,25 @@ static irqreturn_t ipmmu_domain_irq(stru
 static irqreturn_t ipmmu_irq(int irq, void *dev)
 {
struct ipmmu_vmsa_device *mmu = dev;
-   struct iommu_domain *io_domain;
-   struct ipmmu_vmsa_domain *domain;
+   irqreturn_t status = IRQ_NONE;
+   unsigned int i;
+   unsigned long flags;
 
-   if (!mmu->mapping)
-   return IRQ_NONE;
+   spin_lock_irqsave(>lock, flags);
+
+   /*
+* Check interrupts for all active contexts.
+*/
+   for (i = 0; i < IPMMU_CTX_MAX; i++) {
+   if (!mmu->domains[i])
+   continue;
+   if (ipmmu_domain_irq(mmu->domains[i]) == IRQ_HANDLED)
+   status = IRQ_HANDLED;
+   }
 
-   io_domain = mmu->mapping->domain;
-   domain = to_vmsa_domain(io_domain);
+   spin_unlock_irqrestore(>lock, flags);
 
-   

[PATCH v5 00/07] iommu/ipmmu-vmsa: IPMMU multi-arch update V5

[Magnus Damm]

iommu/ipmmu-vmsa: IPMMU multi-arch update V5

[PATCH v5 01/07] iommu/ipmmu-vmsa: Remove platform data handling
[PATCH v5 02/07] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for 
context
[PATCH v5 03/07] iommu/ipmmu-vmsa: Break out utlb parsing code
[PATCH v5 04/07] iommu/ipmmu-vmsa: Break out domain allocation code
[PATCH v5 05/07] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops
[PATCH v5 06/07] iommu/ipmmu-vmsa: ARM and ARM64 archdata access
[PATCH v5 07/07] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency

These patches update the IPMMU driver with a couple of changes
to support build on multiple architectures. In the process of
doing so the interrupt code gets reworked and the foundation
for supporting multiple contexts are added.

Changes since V4:
 - Updated patch 3/7 to work on top on the following commit in next-20160920:
   b1e2afc iommu/ipmmu-vmsa: Fix wrong error handle of ipmmu_add_device
 - Add Kconfig hunk to patch 5/7 to avoid undeclared ipmmu_ops if COMPILE_TEST
 - Rebased patch 7/7 to fit on top of new Kconfig bits in 5/7

Changes since V3:
 - Updated patch 3/7 to fix hang-on-boot issue on 32-bit ARM - thanks Geert!
 - Reworked group parameter handling in patch 3/7 and 5/7.
 - Added patch 6/7 to fix build of the driver on s390/tile/um architectures

Changes since V2:
 - Got rid of patch 3 from the V2 however patch 1, 2 and 4 are kept.
 - V3 patch 3, 4 and 5 come from
   [PATCH 00/04] iommu/ipmmu-vmsa: IPMMU CONFIG_IOMMU_DMA update
 - Patch 5 has been reworked to include patch 3 of the V1 of this series 

Changes since V1:
 - Got rid of patch 2 and 3 from initial series
 - Updated bitmap code locking and also used lighter bitop functions
 - Updated the Kconfig bits to apply on top of ARCH_RENESAS

Signed-off-by: Magnus Damm <damm+rene...@opensource.se>
---

 Built on top next-20160920 as well as the optional fast-track patch
 [PATCH/RFC] iommu/ipmmu-vmsa: Update ->add_device() return value

 drivers/iommu/Kconfig  |2 
 drivers/iommu/ipmmu-vmsa.c |  300 +++-
 2 files changed, 243 insertions(+), 59 deletions(-)

[PATCH v5 07/07] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency

[Magnus Damm]

From: Magnus Damm 

Neither the ARM page table code enabled by IOMMU_IO_PGTABLE_LPAE
nor the IPMMU_VMSA driver actually depends on ARM_LPAE, so get
rid of the dependency.

Tested with ipmmu-vmsa on r8a7794 ALT and a kernel config using:
 # CONFIG_ARM_LPAE is not set

Signed-off-by: Magnus Damm 
Acked-by: Laurent Pinchart 
---

 Changes since V4:
 - Rebased patch to fit on top of earlier Kconfig changes in series

 Changes since V3:
 - None

 Changes since V2:
 - None

 Changes since V1:
 - Rebased on top of ARCH_RENESAS change
 - Added Acked-by from Laurent

 drivers/iommu/Kconfig |1 -
 1 file changed, 1 deletion(-)

--- 0008/drivers/iommu/Kconfig
+++ work/drivers/iommu/Kconfig  2016-09-20 22:13:03.500607110 +0900
@@ -275,7 +275,6 @@ config EXYNOS_IOMMU_DEBUG
 config IPMMU_VMSA
bool "Renesas VMSA-compatible IPMMU"
depends on ARM || IOMMU_DMA
-   depends on ARM_LPAE
depends on ARCH_RENESAS || COMPILE_TEST
select IOMMU_API
select IOMMU_IO_PGTABLE_LPAE

[PATCH v5 06/07] iommu/ipmmu-vmsa: ARM and ARM64 archdata access

[Magnus Damm]

From: Magnus Damm 

Not all architectures have an iommu member in their archdata, so
use #ifdefs support build wit COMPILE_TEST on any architecture.

Signed-off-by: Magnus Damm 
---

 Changes since V4:
 - None

 Changes since V3:
 - New patch

 drivers/iommu/ipmmu-vmsa.c |   37 +++--
 1 file changed, 27 insertions(+), 10 deletions(-)

--- 0012/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-09-20 21:59:21.690607110 +0900
@@ -70,6 +70,25 @@ static struct ipmmu_vmsa_domain *to_vmsa
return container_of(dom, struct ipmmu_vmsa_domain, io_domain);
 }
 
+#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
+static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
+{
+   return dev->archdata.iommu;
+}
+static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
+{
+   dev->archdata.iommu = p;
+}
+#else
+static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
+{
+   return NULL;
+}
+static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
+{
+}
+#endif
+
 #define TLB_LOOP_TIMEOUT   100 /* 100us */
 
 /* 
-
@@ -539,7 +558,7 @@ static void ipmmu_domain_free(struct iom
 static int ipmmu_attach_device(struct iommu_domain *io_domain,
   struct device *dev)
 {
-   struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+   struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
struct ipmmu_vmsa_device *mmu = archdata->mmu;
struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
unsigned long flags;
@@ -581,7 +600,7 @@ static int ipmmu_attach_device(struct io
 static void ipmmu_detach_device(struct iommu_domain *io_domain,
struct device *dev)
 {
-   struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+   struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
unsigned int i;
 
@@ -701,7 +720,7 @@ static int ipmmu_init_platform_device(st
archdata->mmu = mmu;
archdata->utlbs = utlbs;
archdata->num_utlbs = num_utlbs;
-   dev->archdata.iommu = archdata;
+   set_archdata(dev, archdata);
return 0;
 
 error:
@@ -713,12 +732,11 @@ error:
 
 static int ipmmu_add_device(struct device *dev)
 {
-   struct ipmmu_vmsa_archdata *archdata;
struct ipmmu_vmsa_device *mmu = NULL;
struct iommu_group *group;
int ret;
 
-   if (dev->archdata.iommu) {
+   if (to_archdata(dev)) {
dev_warn(dev, "IOMMU driver already assigned to device %s\n",
 dev_name(dev));
return -EINVAL;
@@ -754,8 +772,7 @@ static int ipmmu_add_device(struct devic
 * - Make the mapping size configurable ? We currently use a 2GB mapping
 *   at a 1GB offset to ensure that NULL VAs will fault.
 */
-   archdata = dev->archdata.iommu;
-   mmu = archdata->mmu;
+   mmu = to_archdata(dev)->mmu;
if (!mmu->mapping) {
struct dma_iommu_mapping *mapping;
 
@@ -783,7 +800,7 @@ error:
if (mmu)
arm_iommu_release_mapping(mmu->mapping);
 
-   dev->archdata.iommu = NULL;
+   set_archdata(dev, NULL);
 
if (!IS_ERR_OR_NULL(group))
iommu_group_remove_device(dev);
@@ -793,7 +810,7 @@ error:
 
 static void ipmmu_remove_device(struct device *dev)
 {
-   struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+   struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
 
arm_iommu_detach_device(dev);
iommu_group_remove_device(dev);
@@ -801,7 +818,7 @@ static void ipmmu_remove_device(struct d
kfree(archdata->utlbs);
kfree(archdata);
 
-   dev->archdata.iommu = NULL;
+   set_archdata(dev, NULL);
 }
 
 static struct iommu_domain *ipmmu_domain_alloc(unsigned type)

[PATCH v5 05/07] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops

[Magnus Damm]

From: Magnus Damm 

Introduce an alternative set of iommu_ops suitable for 64-bit ARM
as well as 32-bit ARM when CONFIG_IOMMU_DMA=y. Also adjust the
Kconfig to depend on ARM or IOMMU_DMA.

Signed-off-by: Magnus Damm 
---

 Changes since V4:
 - Added Kconfig hunk to depend on ARM or IOMMU_DMA

 Changes since V3:
 - Removed group parameter from ipmmu_init_platform_device()

 Changes since V2:
 - Included this new patch from the following series:
   [PATCH 00/04] iommu/ipmmu-vmsa: IPMMU CONFIG_IOMMU_DMA update
 - Use only a single iommu_ops structure with #ifdef CONFIG_IOMMU_DMA
 - Folded in #ifdefs to handle CONFIG_ARM and CONFIG_IOMMU_DMA
 - of_xlate() is now used without #ifdefs
 - Made sure code compiles on both 32-bit and 64-bit ARM.

 drivers/iommu/Kconfig  |1 
 drivers/iommu/ipmmu-vmsa.c |  111 
 2 files changed, 104 insertions(+), 8 deletions(-)

--- 0001/drivers/iommu/Kconfig
+++ work/drivers/iommu/Kconfig  2016-09-20 22:10:15.280607110 +0900
@@ -274,6 +274,7 @@ config EXYNOS_IOMMU_DEBUG
 
 config IPMMU_VMSA
bool "Renesas VMSA-compatible IPMMU"
+   depends on ARM || IOMMU_DMA
depends on ARM_LPAE
depends on ARCH_RENESAS || COMPILE_TEST
select IOMMU_API
--- 0006/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-09-20 22:08:58.300607110 +0900
@@ -10,6 +10,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -22,8 +23,10 @@
 #include 
 #include 
 
+#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
 #include 
 #include 
+#endif
 
 #include "io-pgtable.h"
 
@@ -520,14 +523,6 @@ static struct iommu_domain *__ipmmu_doma
return >io_domain;
 }
 
-static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
-{
-   if (type != IOMMU_DOMAIN_UNMANAGED)
-   return NULL;
-
-   return __ipmmu_domain_alloc(type);
-}
-
 static void ipmmu_domain_free(struct iommu_domain *io_domain)
 {
struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
@@ -714,6 +709,8 @@ error:
return ret;
 }
 
+#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
+
 static int ipmmu_add_device(struct device *dev)
 {
struct ipmmu_vmsa_archdata *archdata;
@@ -807,6 +804,14 @@ static void ipmmu_remove_device(struct d
dev->archdata.iommu = NULL;
 }
 
+static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
+{
+   if (type != IOMMU_DOMAIN_UNMANAGED)
+   return NULL;
+
+   return __ipmmu_domain_alloc(type);
+}
+
 static const struct iommu_ops ipmmu_ops = {
.domain_alloc = ipmmu_domain_alloc,
.domain_free = ipmmu_domain_free,
@@ -821,6 +826,94 @@ static const struct iommu_ops ipmmu_ops
.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
 };
 
+#endif /* !CONFIG_ARM && CONFIG_IOMMU_DMA */
+
+#ifdef CONFIG_IOMMU_DMA
+
+static struct iommu_domain *ipmmu_domain_alloc_dma(unsigned type)
+{
+   struct iommu_domain *io_domain;
+
+   if (type != IOMMU_DOMAIN_DMA)
+   return NULL;
+
+   io_domain = __ipmmu_domain_alloc(type);
+   if (io_domain)
+   iommu_get_dma_cookie(io_domain);
+
+   return io_domain;
+}
+
+static void ipmmu_domain_free_dma(struct iommu_domain *io_domain)
+{
+   iommu_put_dma_cookie(io_domain);
+   ipmmu_domain_free(io_domain);
+}
+
+static int ipmmu_add_device_dma(struct device *dev)
+{
+   struct iommu_group *group;
+
+   /* only accept devices with iommus property */
+   if (of_count_phandle_with_args(dev->of_node, "iommus",
+  "#iommu-cells") < 0)
+   return -ENODEV;
+
+   group = iommu_group_get_for_dev(dev);
+   if (IS_ERR(group))
+   return PTR_ERR(group);
+
+   return 0;
+}
+
+static void ipmmu_remove_device_dma(struct device *dev)
+{
+   iommu_group_remove_device(dev);
+}
+
+static struct iommu_group *ipmmu_device_group_dma(struct device *dev)
+{
+   struct iommu_group *group;
+   int ret;
+
+   group = generic_device_group(dev);
+   if (IS_ERR(group))
+   return group;
+
+   ret = ipmmu_init_platform_device(dev);
+   if (ret) {
+   iommu_group_put(group);
+   group = ERR_PTR(ret);
+   }
+
+   return group;
+}
+
+static int ipmmu_of_xlate_dma(struct device *dev,
+ struct of_phandle_args *spec)
+{
+   /* dummy callback to satisfy of_iommu_configure() */
+   return 0;
+}
+
+static const struct iommu_ops ipmmu_ops = {
+   .domain_alloc = ipmmu_domain_alloc_dma,
+   .domain_free = ipmmu_domain_free_dma,
+   .attach_dev = ipmmu_attach_device,
+   .detach_dev = ipmmu_detach_device,
+   .map = ipmmu_map,
+   .unmap = ipmmu_unmap,
+   .map_sg = default_iommu_map_sg,
+   .iova_to_phys = ipmmu_iova_to_phys,
+   .add_device = ipmmu_add_device_dma,
+   .remove_device = 

[renesas-drivers:topic/gen3-latest 12/52] drivers/gpu/drm/i915/intel_ddi.c:453:3: error: 'n_hdmi_entries' undeclared

[kbuild test robot]

tree:   
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git 
topic/gen3-latest
head:   c78265667b735cc07e53731abd2140d1bf4e7a41
commit: 8c26677f9f655eea278f4fc5395a3a0251db9c65 [12/52] Merge remote-tracking 
branch 'drm/drm-next' into renesas-drivers
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout 8c26677f9f655eea278f4fc5395a3a0251db9c65
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_ddi.c: In function 'intel_prepare_dp_ddi_buffers':
>> drivers/gpu/drm/i915/intel_ddi.c:453:3: error: 'n_hdmi_entries' undeclared 
>> (first use in this function)
  n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
  ^~
   drivers/gpu/drm/i915/intel_ddi.c:453:3: note: each undeclared identifier is 
reported only once for each function it appears in

vim +/n_hdmi_entries +453 drivers/gpu/drm/i915/intel_ddi.c

f8896f5d David Weinehall 2015-06-25  437ddi_translations_edp =
78ab0bae Ville Syrjälä   2015-12-08  438
skl_get_buf_trans_edp(dev_priv, _edp_entries);
32bdc400 Ville Syrjälä   2016-07-12  439  
75067dde Antti Koskipaa  2015-07-10  440/* If we're boosting 
the current, set bit 31 of trans1 */
1edaaa2f Ville Syrjälä   2016-07-12  441if 
(dev_priv->vbt.ddi_port_info[port].dp_boost_level)
c110ae6c Ville Syrjälä   2016-07-12  442iboost_bit = 
DDI_BUF_BALANCE_LEG_ENABLE;
10afa0b6 Ville Syrjälä   2015-12-08  443  
ceccad59 Ville Syrjälä   2016-01-12  444if 
(WARN_ON(encoder->type == INTEL_OUTPUT_EDP &&
ceccad59 Ville Syrjälä   2016-01-12  445port != 
PORT_A && port != PORT_E &&
ceccad59 Ville Syrjälä   2016-01-12  446
n_edp_entries > 9))
10afa0b6 Ville Syrjälä   2015-12-08  447n_edp_entries = 
9;
78ab0bae Ville Syrjälä   2015-12-08  448} else if 
(IS_BROADWELL(dev_priv)) {
e58623cb Art Runyan  2013-11-02  449ddi_translations_fdi = 
bdw_ddi_translations_fdi;
e58623cb Art Runyan  2013-11-02  450ddi_translations_dp = 
bdw_ddi_translations_dp;
a930acd9 Ville Syrjälä   2016-07-12  451ddi_translations_edp = 
bdw_get_buf_trans_edp(dev_priv, _edp_entries);
7ad14a29 Sonika Jindal   2015-02-25  452n_dp_entries = 
ARRAY_SIZE(bdw_ddi_translations_dp);
10122051 Jani Nikula 2014-08-27 @453n_hdmi_entries = 
ARRAY_SIZE(bdw_ddi_translations_hdmi);
78ab0bae Ville Syrjälä   2015-12-08  454} else if 
(IS_HASWELL(dev_priv)) {
e58623cb Art Runyan  2013-11-02  455ddi_translations_fdi = 
hsw_ddi_translations_fdi;
e58623cb Art Runyan  2013-11-02  456ddi_translations_dp = 
hsw_ddi_translations_dp;
300644c7 Paulo Zanoni2013-11-02  457ddi_translations_edp = 
hsw_ddi_translations_dp;
7ad14a29 Sonika Jindal   2015-02-25  458n_dp_entries = 
n_edp_entries = ARRAY_SIZE(hsw_ddi_translations_dp);
10122051 Jani Nikula 2014-08-27  459n_hdmi_entries = 
ARRAY_SIZE(hsw_ddi_translations_hdmi);
e58623cb Art Runyan  2013-11-02  460} else {
e58623cb Art Runyan  2013-11-02  461WARN(1, "ddi 
translation table missing\n");

:: The code at line 453 was first introduced by commit
:: 1012205182fb9470a1bd1620872103a09f566225 drm/i915/ddi: use struct for 
ddi buf translation tables

:: TO: Jani Nikula 
:: CC: Daniel Vetter 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH/RFC] iommu/ipmmu-vmsa: Update ->add_device() return value

[Robin Murphy]

Hi Magnus,

On 20/09/16 13:41, Magnus Damm wrote:
> From: Magnus Damm <damm+rene...@opensource.se>
> 
> Update the IPMMU driver to return -ENODEV when adding devices
> not hooked up a particular IPMMU instance.
> 
> Currently the ->add_device() callback implementation in the IPMMU
> driver returns -ENODEV for devices with no "iommus" property,
> however the function ipmmu_find_utlbs() may return -EINVAL.

If there were no "iommus" property at all, of_parse_phandle_with_args()
should return -ENOENT - that probably does want to be caught and passed
back as -ENODEV to imply an untranslated device. On the other hand,
-EINVAL would stem from the existence of the property, but in a somehow
erroneous manner - other than the "args.np != mmu->dev->of_node" check
(which could legitimately fail and be safely ignored if there are
multiple IOMMUs in the system), any other reason implies a DT error
which probably shouldn't be papered over.

Robin.

> This patch updates the ipmmu_find_utlbs() return value to -ENODEV
> for the case when multiple IPMMU instances exist. That way the
> code matches the expected behaviour described in the comment of
> the add_iommu_group() function in iommu.c:
> 
>  /*
>   * We ignore -ENODEV errors for now, as they just mean that the
>   * device is not translated by an IOMMU. We still care about
>   * other errors and fail to initialize when they happen.
>   */
> 
> Signed-off-by: Magnus Damm <damm+rene...@opensource.se>
> ---
> 
>  Applies to next-20160920 on top of:
>  b1e2afc iommu/ipmmu-vmsa: Fix wrong error handle of ipmmu_add_device
> 
>  drivers/iommu/ipmmu-vmsa.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- 0002/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c   2016-09-08 18:20:06.270607110 +0900
> @@ -781,7 +781,7 @@ static int ipmmu_find_utlbs(struct ipmmu
>   of_node_put(args.np);
>  
>   if (args.np != mmu->dev->of_node || args.args_count != 1)
> - return -EINVAL;
> + return -ENODEV;
>  
>   utlbs[i] = args.args[0];
>   }
> ___
> iommu mailing list
> io...@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
> 

[PATCH/RFC] iommu/ipmmu-vmsa: Update ->add_device() return value

[Magnus Damm]

From: Magnus Damm <damm+rene...@opensource.se>

Update the IPMMU driver to return -ENODEV when adding devices
not hooked up a particular IPMMU instance.

Currently the ->add_device() callback implementation in the IPMMU
driver returns -ENODEV for devices with no "iommus" property,
however the function ipmmu_find_utlbs() may return -EINVAL.

This patch updates the ipmmu_find_utlbs() return value to -ENODEV
for the case when multiple IPMMU instances exist. That way the
code matches the expected behaviour described in the comment of
the add_iommu_group() function in iommu.c:

 /*
  * We ignore -ENODEV errors for now, as they just mean that the
  * device is not translated by an IOMMU. We still care about
  * other errors and fail to initialize when they happen.
  */

Signed-off-by: Magnus Damm <damm+rene...@opensource.se>
---

 Applies to next-20160920 on top of:
 b1e2afc iommu/ipmmu-vmsa: Fix wrong error handle of ipmmu_add_device

 drivers/iommu/ipmmu-vmsa.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- 0002/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-09-08 18:20:06.270607110 +0900
@@ -781,7 +781,7 @@ static int ipmmu_find_utlbs(struct ipmmu
of_node_put(args.np);
 
if (args.np != mmu->dev->of_node || args.args_count != 1)
-   return -EINVAL;
+   return -ENODEV;
 
utlbs[i] = args.args[0];
}

[PATCH] arm64: dts: r8a7796: salvator-x: Populate EXTALR

[Geert Uytterhoeven]

It can be used for the watchdog.

Based on similar work for r8a7795/salvator-x by Wolfram Sang.

Signed-off-by: Geert Uytterhoeven 
---
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts 
b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
index 13db7d61c26c28b8..90e9a76c8b3080d7 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
@@ -49,6 +49,10 @@
clock-frequency = <1666>;
 };
 
+_clk {
+   clock-frequency = <32768>;
+};
+
  {
pinctrl-0 = <_pins>;
pinctrl-names = "default";
-- 
1.9.1

[PATCH/RFC] ARM: dts: r8a7790: IPMMU-DS SYS-DMAC prototype

[Magnus Damm]

From: Magnus Damm 

Hook up the SYS-DMAC devices to IPMMU-DS on r8a7790 and enable that
particular IPMMU instance. Depending on kernel version LPAE may need
to be enabled before the IPMMU driver becomes available.

Useful to test the IPMMU with the devices hooked up to the SYS-DMAC
via the DMA Engine framework together with the following workaround:
[PATCH/RFC] iommu/ipmmu-vmsa: IPMMU SYS-DMAC iova mapping workaround

Signed-off-by: Magnus Damm 
---

 Tested with renesas-drivers-2016-09-13-v4.8-rc6

 arch/arm/boot/dts/r8a7790.dtsi |   31 ++-
 1 file changed, 30 insertions(+), 1 deletion(-)

--- 0003/arch/arm/boot/dts/r8a7790.dtsi
+++ work/arch/arm/boot/dts/r8a7790.dtsi 2016-09-20 20:23:34.420607110 +0900
@@ -362,6 +362,21 @@
power-domains = < R8A7790_PD_ALWAYS_ON>;
#dma-cells = <1>;
dma-channels = <15>;
+   iommus = <_ds 0>,
+  <_ds 1>,
+  <_ds 2>,
+  <_ds 3>,
+  <_ds 4>,
+  <_ds 5>,
+  <_ds 6>,
+  <_ds 7>,
+  <_ds 8>,
+  <_ds 9>,
+  <_ds 10>,
+  <_ds 11>,
+  <_ds 12>,
+  <_ds 13>,
+  <_ds 14>;
};
 
dmac1: dma-controller@e672 {
@@ -393,6 +408,21 @@
power-domains = < R8A7790_PD_ALWAYS_ON>;
#dma-cells = <1>;
dma-channels = <15>;
+   iommus = <_ds 15>,
+  <_ds 16>,
+  <_ds 17>,
+  <_ds 18>,
+  <_ds 19>,
+  <_ds 20>,
+  <_ds 21>,
+  <_ds 22>,
+  <_ds 23>,
+  <_ds 24>,
+  <_ds 25>,
+  <_ds 26>,
+  <_ds 27>,
+  <_ds 28>,
+  <_ds 29>;
};
 
audma0: dma-controller@ec70 {
@@ -1877,7 +1907,6 @@
interrupts = ,
 ;
#iommu-cells = <1>;
-   status = "disabled";
};
 
ipmmu_mp: mmu@ec68 {

[PATCH/RFC] ARM: dts: r8a7790: IPMMU-MX DU prototype

[Magnus Damm]

From: Magnus Damm 

These two hunks of code enable the r8a7790 IPMMU-MX instance together with
the DU device on r8a7790. Depending on kernel version LPAE may need to
be enabled before the IPMMU driver becomes available.

Useful to test the IPMMU with the DU via the VGA port on r8a7790 Lager.

# modetest -M rcar-du -s 60:800x600

Not-Yet-Signed-off-by: Magnus Damm 
---

 Tested with renesas-drivers-2016-09-13-v4.8-rc6

 arch/arm/boot/dts/r8a7790.dtsi |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- 0001/arch/arm/boot/dts/r8a7790.dtsi
+++ work/arch/arm/boot/dts/r8a7790.dtsi 2016-09-20 20:17:26.0 +0900
@@ -985,6 +985,7 @@
 <_clks R8A7790_CLK_LVDS0>,
 <_clks R8A7790_CLK_LVDS1>;
clock-names = "du.0", "du.1", "du.2", "lvds.0", "lvds.1";
+   iommus = <_mx 15>, <_mx 16>;
status = "disabled";
 
ports {
@@ -1893,7 +1894,7 @@
interrupts = ,
 ;
#iommu-cells = <1>;
-   status = "disabled";
+   status = "okay";
};
 
ipmmu_rt: mmu@ffc8 {

[PATCH/RFC] iommu/ipmmu-vmsa: IPMMU SYS-DMAC iova mapping workaround

[Magnus Damm]

From: Magnus Damm 

Here's some prototype code that works around the lack of software
support for mapping I/O devices to the SYS-DMAC hardware via the
DMA Engine framework when using IOMMU.

The code itself is one big layering violation that goes through
the DT and unconditionally maps I/O devices using DMACs via the
IPMMU device instance into iova space with a 1:1 mapping.

This very short term prototype will for instance automatically make
the SCIF serial port function with the IPMMU hardware in case the
SYS-DMAC is hooked up to the IPMMU device.

Not to be confused with the more long term solution to allow the
DMA Engine framework to map I/O device memory dynamically.

Not-Yet-Signed-off-by: Magnus Damm 
---

 Applies on top of: renesas-drivers-2016-09-13-v4.8-rc6

 drivers/iommu/ipmmu-vmsa.c |   75 
 1 file changed, 75 insertions(+)

--- 0001/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-09-20 20:03:37.620607110 +0900
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -625,6 +626,78 @@ static void ipmmu_domain_free(struct iom
kfree(domain);
 }
 
+static void ipmmu_workaround_map(struct iommu_domain *io_domain,
+struct device *dma_dev, struct resource *res)
+{
+   phys_addr_t phys_addr;
+
+   dev_info(dma_dev, "map %pr\n", res);
+
+   phys_addr = iommu_iova_to_phys(io_domain, res->start);
+   if (phys_addr)
+   return;
+
+   iommu_map(io_domain, res->start, res->start,
+ ALIGN(resource_size(res), SZ_4K),
+ IOMMU_READ | IOMMU_WRITE);
+}
+
+static void ipmmu_workaround_dt(struct iommu_domain *io_domain,
+   struct device *dev,
+   void (*match)(struct iommu_domain *io_domain,
+ struct device *dma_dev,
+ struct resource *res))
+{
+   struct device_node *np = NULL;
+   struct of_phandle_args dma_spec;
+   struct resource r;
+   int i, cnt;
+   bool found;
+
+   /* Locate I/O devices using the DMAC and map their registers */
+   while ((np = of_find_all_nodes(np))) {
+   if (!of_find_property(np, "dmas", NULL))
+   continue;
+
+   cnt = of_property_count_strings(np, "dma-names");
+   if (cnt < 0)
+   continue;
+
+   found = false;
+   for (i = 0; i < cnt; i++) {
+   if (of_parse_phandle_with_args(np, "dmas",
+  "#dma-cells", i,
+  _spec))
+   continue;
+
+   if (dma_spec.np == dev->of_node)
+   found = true;
+
+   of_node_put(dma_spec.np);
+   }
+
+   if (!found)
+   continue;
+
+   i = 0;
+   while (!of_address_to_resource(np, i, )) {
+   match(io_domain, dev, );
+   i++;
+   }
+   }
+}
+
+static void ipmmu_workaround(struct iommu_domain *io_domain,
+struct device *dev)
+{
+   /* only apply workaround for DMA controllers */
+   if (!strstr(dev_name(dev), "dma-controller"))
+   return;
+
+   dev_info(dev, "Adding iommu workaround to map I/O devices for DMACs\n");
+   ipmmu_workaround_dt(io_domain, dev, ipmmu_workaround_map);
+}
+
 static int ipmmu_attach_device(struct iommu_domain *io_domain,
   struct device *dev)
 {
@@ -678,6 +751,8 @@ static int ipmmu_attach_device(struct io
if (ret < 0)
return ret;
 
+   ipmmu_workaround(io_domain, dev);
+
for (i = 0; i < archdata->num_utlbs; ++i)
ipmmu_utlb_enable(domain, archdata->utlbs[i]);
 

Re: [PATCH v3 4/6] mmc: tmio: add eMMC support

[Jaehoon Chung]

On 09/20/2016 05:57 AM, Wolfram Sang wrote:
> We need to add R1 without CRC support, refactor the bus width routine a
> little and extend a quirk check. To support "non-removable;" we need a
> workaround which will be hopefully removed when reworking PM soon.
> 
> Signed-off-by: Wolfram Sang 
> ---
>  drivers/mmc/host/tmio_mmc.h |  3 +++
>  drivers/mmc/host/tmio_mmc_pio.c | 38 ++
>  2 files changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 4b501f2d529f6e..637581faf756b1 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -79,6 +79,9 @@
>  #define  CLK_CTL_DIV_MASK0xff
>  #define  CLK_CTL_SCLKEN  BIT(8)
>  
> +#define CARD_OPT_WIDTH8  BIT(13)
> +#define CARD_OPT_WIDTH   BIT(15)

Just confusing whether CARD_OPT_WIDTH is 4bit or 1bit?

> +
>  #define TMIO_BBS 512 /* Boot block size */
>  
>  /* Definitions for values the CTRL_SDIO_STATUS register can take. */
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index 46b5a456243b84..a0f05eb4f34490 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -340,7 +340,9 @@ static int tmio_mmc_start_command(struct tmio_mmc_host 
> *host, struct mmc_command
>  
>   switch (mmc_resp_type(cmd)) {
>   case MMC_RSP_NONE: c |= RESP_NONE; break;
> - case MMC_RSP_R1:   c |= RESP_R1;   break;
> + case MMC_RSP_R1:
> + case MMC_RSP_R1_NO_CRC:
> +c |= RESP_R1;   break;

Just wonder..It there case that hit "case MMC_RSP_R1_NO_CRC" ?

>   case MMC_RSP_R1B:  c |= RESP_R1B;  break;
>   case MMC_RSP_R2:   c |= RESP_R2;   break;
>   case MMC_RSP_R3:   c |= RESP_R3;   break;
> @@ -737,12 +739,13 @@ static int tmio_mmc_start_data(struct tmio_mmc_host 
> *host,
>   pr_debug("setup data transfer: blocksize %08x  nr_blocks %d\n",
>data->blksz, data->blocks);
>  
> - /* Some hardware cannot perform 2 byte requests in 4 bit mode */
> - if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
> + /* Some hardware cannot perform 2 byte requests in 4/8 bit mode */
> + if (host->mmc->ios.bus_width == MMC_BUS_WIDTH_4 ||
> + host->mmc->ios.bus_width == MMC_BUS_WIDTH_8) {
>   int blksz_2bytes = pdata->flags & TMIO_MMC_BLKSZ_2BYTES;
>  
>   if (data->blksz < 2 || (data->blksz < 4 && !blksz_2bytes)) {
> - pr_err("%s: %d byte block unsupported in 4 bit mode\n",
> + pr_err("%s: %d byte block unsupported in 4/8 bit 
> mode\n",
>  mmc_hostname(host->mmc), data->blksz);
>   return -EINVAL;
>   }
> @@ -922,14 +925,16 @@ static void tmio_mmc_power_off(struct tmio_mmc_host 
> *host)
>  static void tmio_mmc_set_bus_width(struct tmio_mmc_host *host,
>   unsigned char bus_width)
>  {
> - switch (bus_width) {
> - case MMC_BUS_WIDTH_1:
> - sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x80e0);
> - break;
> - case MMC_BUS_WIDTH_4:
> - sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x00e0);
> - break;
> - }
> + u16 reg = sd_ctrl_read16(host, CTL_SD_MEM_CARD_OPT)
> + & ~(CARD_OPT_WIDTH | CARD_OPT_WIDTH8);
> +
> + /* reg now applies to MMC_BUS_WIDTH_4 */
> + if (bus_width == MMC_BUS_WIDTH_1)
> + reg |= CARD_OPT_WIDTH;
> + else if (bus_width == MMC_BUS_WIDTH_8)
> + reg |= CARD_OPT_WIDTH8;
> +
> + sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, reg);
>  }
>  
>  /* Set MMC clock / power.
> @@ -1149,6 +1154,15 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
> !mmc_card_is_removable(mmc) ||
> mmc->slot.cd_irq >= 0);
>  
> + /*
> +  * On Gen2+, eMMC with NONREMOVABLE currently fails because native
> +  * hotplug gets disabled. It seems RuntimePM related yet we need further
> +  * research. Since we are planning a PM overhaul anyway, let's enforce
> +  * for now the device being active by enabling native hotplug always.
> +  */
> + if (pdata->flags & TMIO_MMC_MIN_RCAR2)
> + _host->native_hotplug = true;
> +
>   if (tmio_mmc_clk_enable(_host) < 0) {
>   mmc->f_max = pdata->hclk;
>   mmc->f_min = mmc->f_max / 512;
> 

Re: [PATCH v3 2/6] mmc: rtsx_pci: use new macro for R1 without CRC

[Jaehoon Chung]

Hi Wolfram,

Add the commit message.

On 09/20/2016 05:57 AM, Wolfram Sang wrote:
> Signed-off-by: Wolfram Sang 
> ---
>  drivers/mmc/host/rtsx_pci_sdmmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c 
> b/drivers/mmc/host/rtsx_pci_sdmmc.c
> index 396c9b7e4121b0..3ccaa1415f33b2 100644
> --- a/drivers/mmc/host/rtsx_pci_sdmmc.c
> +++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
> @@ -126,7 +126,7 @@ static int sd_response_type(struct mmc_command *cmd)
>   return SD_RSP_TYPE_R0;
>   case MMC_RSP_R1:
>   return SD_RSP_TYPE_R1;
> - case MMC_RSP_R1 & ~MMC_RSP_CRC:
> + case MMC_RSP_R1_NO_CRC:
>   return SD_RSP_TYPE_R1 | SD_NO_CHECK_CRC7;
>   case MMC_RSP_R1B:
>   return SD_RSP_TYPE_R1b;
> 

Re: [PATCH v3 1/6] mmc: add define for R1 response without CRC

[Jaehoon Chung]

Hi Wolfram,

On 09/20/2016 05:57 AM, Wolfram Sang wrote:
> The core uses it for polling. Give drivers a proper define handle this
> case like for other response types.
> 
> Signed-off-by: Wolfram Sang 
> ---
>  include/linux/mmc/core.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index b01e77de1a74de..4caee099b63a28 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -55,6 +55,9 @@ struct mmc_command {
>  #define MMC_RSP_R6   (MMC_RSP_PRESENT|MMC_RSP_CRC|MMC_RSP_OPCODE)
>  #define MMC_RSP_R7   (MMC_RSP_PRESENT|MMC_RSP_CRC|MMC_RSP_OPCODE)
>  
> +/* Can be used by core to poll after switch to MMC HS mode */
> +#define MMC_RSP_R1_NO_CRC(MMC_RSP_PRESENT|MMC_RSP_OPCODE)

MMC_RSP_RQ_NO_CRC is described at Specification?

Best Regards,
Jaehoon Chung

> +
>  #define mmc_resp_type(cmd)   ((cmd)->flags & 
> (MMC_RSP_PRESENT|MMC_RSP_136|MMC_RSP_CRC|MMC_RSP_BUSY|MMC_RSP_OPCODE))
>  
>  /*
> 

Re: [PATCH 0/4] ARM: dts: rcar-gen2: Correct SCIFB reg properties to cover all

[Simon Horman]

On Mon, Sep 19, 2016 at 04:18:52PM +0200, Geert Uytterhoeven wrote:
>   Hi Simon, Magnus,
> 
> Several SCIFB registers reside outside the register ranges as specified
> by the "reg" properties of the various R-Car Gen2 DTSes.  Fortunately
> this works (on Linux), due to the PAGE_SIZE granularity of ioremap().
> 
> Extend the sizes from 64 to 0x100 bytes to fix this, like is done on
> SH/R-Mobile SoCs.

Thanks, I have queued these up.

Re: [PATCH 1/3] clk: shmobile: r8a7796: Add SCIF clocks

[Geert Uytterhoeven]

On Thu, Sep 15, 2016 at 8:30 PM, Geert Uytterhoeven
 wrote:
> On Wed, Sep 14, 2016 at 6:46 PM, Ulrich Hecht
>  wrote:
>> Signed-off-by: Ulrich Hecht 
>
> Reviewed-by: Geert Uytterhoeven 

Queueing in clk-renesas-for-v4.10 with s/shmobile/renesas/.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 1/3] clk: renesas: r8a7796: Add SYS-DMAC clocks

[Geert Uytterhoeven]

On Thu, Sep 15, 2016 at 1:19 PM, Geert Uytterhoeven
 wrote:
> On Wed, Sep 14, 2016 at 6:45 PM, Ulrich Hecht
>  wrote:
>> Signed-off-by: Ulrich Hecht 
>> ---
>>  drivers/clk/renesas/r8a7796-cpg-mssr.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/clk/renesas/r8a7796-cpg-mssr.c 
>> b/drivers/clk/renesas/r8a7796-cpg-mssr.c
>> index eb347ed..c02fe34 100644
>> --- a/drivers/clk/renesas/r8a7796-cpg-mssr.c
>> +++ b/drivers/clk/renesas/r8a7796-cpg-mssr.c
>> @@ -109,6 +109,9 @@ static const struct cpg_core_clk r8a7796_core_clks[] 
>> __initconst = {
>>  };
>>
>>  static const struct mssr_mod_clk r8a7796_mod_clks[] __initconst = {
>> +   DEF_MOD("sys-dmac2", 217,   R8A7796_CLK_S3D1),
>> +   DEF_MOD("sys-dmac1", 218,   R8A7796_CLK_S3D1),
>> +   DEF_MOD("sys-dmac0", 219,   R8A7796_CLK_S3D1),
>
> It's not clear from the documentation what the actual parent clock is.
> The datasheet says "ZS", which we know is S3D1 on H3.
> However, Table 50.2 says ZS is S0D3 on M3-W (and H3 ES2.0)

Queuing in clk-renesas-for-v4.10 with parent clock fixed to S0D3.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] [media] vsp1: fix CodingStyle violations on multi-line comments

[Laurent Pinchart]

Hi Mauro,

On Monday 19 Sep 2016 16:10:31 Mauro Carvalho Chehab wrote:
> Em Mon, 19 Sep 2016 21:35:36 +0300 Laurent Pinchart escreveu:
> > On Monday 19 Sep 2016 15:26:19 Mauro Carvalho Chehab wrote:
> >> Several multi-line comments added at the vsp1 patch series
> >> violate the Kernel CodingStyle. Fix them.
> >> 
> >> Signed-off-by: Mauro Carvalho Chehab 
> > 
> > I prefer the current style but that seems to be a hopeless battle :-) I
> > have a small comment, please see below.
> > 
> >> ---
> >> 
> >>  drivers/media/platform/vsp1/vsp1_bru.c|  3 ++-
> >>  drivers/media/platform/vsp1/vsp1_clu.c|  3 ++-
> >>  drivers/media/platform/vsp1/vsp1_dl.c | 21 ++---
> >>  drivers/media/platform/vsp1/vsp1_drm.c|  3 ++-
> >>  drivers/media/platform/vsp1/vsp1_entity.h |  2 +-
> >>  drivers/media/platform/vsp1/vsp1_pipe.c   |  2 +-
> >>  drivers/media/platform/vsp1/vsp1_rpf.c|  9 ++---
> >>  drivers/media/platform/vsp1/vsp1_rwpf.c   |  6 --
> >>  drivers/media/platform/vsp1/vsp1_video.c  | 20 +---
> >>  drivers/media/platform/vsp1/vsp1_wpf.c|  9 ++---
> >>  10 files changed, 51 insertions(+), 27 deletions(-)

[snip]

> >> diff --git a/drivers/media/platform/vsp1/vsp1_entity.h
> >> b/drivers/media/platform/vsp1/vsp1_entity.h index
> >> 90a4d95c0a50..901146f807b9 100644
> >> --- a/drivers/media/platform/vsp1/vsp1_entity.h
> >> +++ b/drivers/media/platform/vsp1/vsp1_entity.h
> >> @@ -35,7 +35,7 @@ enum vsp1_entity_type {
> >>VSP1_ENTITY_WPF,
> >>  };
> >> 
> >> -/*
> >> +/**
> > 
> > Quoting another mail I've sent:
> > 
> > I don't think those comments should become part of the kernel
> > documentation. They're really about driver internals, and meant for the
> > driver developers. In particular only a subset of the driver is
> > documented that way, when I've considered that the code or structures
> > were complex enough to need proper documentation. A generated doc would
> > then be quite incomplete and not very useful, the comments are meant to
> > be read while working on the code.
>
> Just doing the above won't make it part of the Kernel documentation.
> 
> It will only be part of it if you explicitly include the file with
> the ".. kernel-doc::" directive.
> 
> Even if you don't add it at the Kernel documentation, I strongly
> suggest to use the kernel-doc tags and format, due to two reasons:
> 
> 1) If you later want to add a book, there's no need to touch at the
> function/struct documentation. Everything will there already;
> 
> 2) Markus Raiser is writing validation tool for those tags:
>   install: https://return42.github.io/linuxdoc/install.html
>   lint:https://return42.github.io/linuxdoc/cmd-line.html#kernel-> 
> lintdoc
> 
> By using his tool, you would be able to check if a patch is keeping
> the documentation documented, as you modify it.

Ah, I wasn't aware of that validation tool. That's a very good point. Given 
that the documentation will not be generated by switching to /** only I agree 
with you that using that tag is a good idea.

Thanks for the patch again.

> Btw, on several places inside the vsp1 documentation, you're using the
> "/**" tag already for other function/struct descriptions.

-- 
Regards,

Laurent Pinchart