Re: [PATCH v1 00/25] PCI: Request host bridge window resources

[Arnd Bergmann]

On Monday, June 6, 2016 6:04:44 PM CEST Bjorn Helgaas wrote:
> Several host bridge drivers (designware and all derivatives, iproc,
> xgene, xilinx, and xilinx-nwl) don't request the MMIO and I/O port
> windows they forward downstream to the PCI bus.
> 
> That means the PCI core can't request resources for PCI bridge
> windows and PCI BARs.
> 
> Several other drivers (altera, generic, mvebu, rcar, tegra) do request
> the windows, but use some duplicated code to do it.
> 
> This adds a new devm_request_pci_bus_resources() interface and changes
> these drivers to use it.  It also fixes several error paths where we failed
> to free the resource list allocated by of_pci_get_host_bridge_resources().
> 
> Tegra guys, please take a look at "PCI: tegra: Remove top-level resource
> from hierarchy" in particular.  Removing the top-level resource definitely
> makes /proc/iomem look uglier (although it will look more like that of
> other drivers).  A short-term fix could be to include device information in
> the resource name.  I think a better long-term fix would be to make the DT
> or platform device core request all the resources from the DT.
> 
> Comments welcome.  I expect we'll trip over something here, so I marked
> this "v1" and I don't plan to put it into -next for a while.
> 
> This is on my pci/host-request-windows branch, which you can pull or view
> at 
> https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-request-windows

This looks very nice. There is one related aspect that I have been
grumbling about for a while, but I don't know what the driver is
actually supposed to do there:

For the IORESOURCE_IO resources, some drivers request the MMIO address
that the window is mapped into, some drivers request the PIO range, and
some of them request both. I also believe the resource that gets put
into the bridge resources list is not always the same one (or maybe
that got fixed by now).

What do you think is the correct behavior here, should the driver only
request the PIO range with parent=ioport_resource, or should it also
request the MMIO window for the I/O ports with parent=iomem_resource?
In the latter case, any idea how that can be generalized?

Another aspect is that we already have the
gen_pci_parse_request_of_pci_ranges() function that does the same as your
new devm_request_pci_bus_resources() and then a few other things. I
have been wondering whether we could move that function into common
code convert drivers to use that wherever possible, but I guess we can
always do that as a follow-up after this series.

Arnd

Re: [PATCH v1 00/25] PCI: Request host bridge window resources

[Arnd Bergmann]

On Tuesday, June 7, 2016 8:11:05 AM CEST Bjorn Helgaas wrote:
> > 
> > What do you think is the correct behavior here, should the driver only
> > request the PIO range with parent=ioport_resource, or should it also
> > request the MMIO window for the I/O ports with parent=iomem_resource?
> > In the latter case, any idea how that can be generalized?
> 
> I think it should request both because I think iomem_resource should
> contain everything in the memory map.  This would be required if we ever
> did any significant reassignment of top-level devices, e.g., ACPI devices.

Ok. Should we try to pass the mmio resource for the I/O window to
the devm_request_pci_bus_resources() function along with the other
arguments then?

As far as I can tell, it should not go into the resource list
because it is not something the PCI core code should access the
way it handles the other resources.

Arnd

Re: [GIT PULL v2] Renesas ARM Based SoC Updates for v4.6

[Arnd Bergmann]

On Wednesday 24 February 2016 10:21:25 Simon Horman wrote:
> Renesas ARM Based SoC Updates for v4.6
> 
> * Enable PM and PM_GENERIC_DOMAINS for SoCs with PM Domains
> * Move emev2_smp_ops to emev2
> * Remove legacy map_io callbacks on r8a7740 and emev2 SoCs
> * Migrate to generic l2c OF initialization on r8a7740
> 

Pulled into next/soc, thanks! I guess some of it could have been
part of the cleanup branch as well, but it seems fine either way.

Arnd

Re: [GIT PULL] Second Round of Renesas ARM64 Based SoC DT Updates for v4.6

[Arnd Bergmann]

On Friday 26 February 2016 09:07:32 Simon Horman wrote:
> Please consider these second round of Renesas ARM64 based SoC DT updates
> for v4.6.
> 
> This pull request is based on the previous round of
> such requests, tagged as renesas-arm64-dt-for-v4.6,
> which you have already pulled.


Pulled into next/dt64, thanks!

Arnd

Re: [GIT PULL] Renesas ARM64 Based SoC SoC Updates for v4.6

[Arnd Bergmann]

On Friday 26 February 2016 08:52:55 Simon Horman wrote:
> Renesas ARM64 Based SoC SoC Updates for v4.6
> 
> * Enable RENESAS_IRQC, and PM and PM_GENERIC_DOMAINS for SoCs with PM Domains
> 

Pulled into next/arm64, thanks!

Arnd

Re: [GIT PULL] Renesas ARM64 Based SoC Defconfig Updates for v4.6

[Arnd Bergmann]

On Friday 26 February 2016 08:51:45 Simon Horman wrote:
> Please consider these Renesas ARM64 based SoC defconfig updates for v4.6.
> 
> This pull request is based on the previous round of
> such requests, tagged as v4.5-rc2,
> which I have already sent a pull-request for.

It looks like your script generated a funny text here, as you
did not send -rc2 ;-)

> I based this pull request on v4.5-rc2 instead of rc1 to avoid
> a minor conflict. However I believe this may have been in vein
> as there appears to be a separate minor conflict present in linux-next.

Doesn't really matter here, the trivial conflicts are never a
problem, and the branches are already based on -rc3, so you are
not introducing a backmerge.

Pulled into next/arm64, thanks!

Arnd

Re: [GIT PULL] Second Round of Renesas ARM Based SoC DT Updates for v4.6

[Arnd Bergmann]

On Friday 26 February 2016 08:53:07 Simon Horman wrote:
> Second Round of Renesas ARM Based SoC DT Updates for v4.6
> 
> * Add L2 cache-controller nodes to r8a779[0134] and r8a73a4
> * Add etheravb support to r8a7794
> * Correct JP3 jumper description on Porter
> * Enable thermal zone on  r8a779[013]
> * Replace gpio-key, wakeup with wakeup-source property on r8a7794
> * Use demuxer for IIC0/I2C0 on lager
> * Use fallback etheravb, pci and pcie compatibility strings as appropriate
> 
> 

Merged into next/dt, thanks!

Arnd

[PATCH] drm: rcar-du: clarify vsp dependency

[Arnd Bergmann]

The VSP1 compositor code in DRM links against the respective V4L
driver, but the dependency is not expressed correctly in Kconfig,
which leads to a build error when the DRM driver is built-in
and the V4L driver is a module:

drivers/gpu/built-in.o: In function `rcar_du_vsp_plane_atomic_update':
rcar-du/rcar_du_vsp.c:183: undefined reference to `vsp1_du_atomic_update'

This patch avoids the problem by ensuring that the DRM VSP code can
only be enabled if the V4L driver is linked into the kernel, or
both are loadable modules.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
Fixes: 6d62ef3ac30b ("drm: rcar-du: Expose the VSP1 compositor through KMS 
planes")
---
 drivers/gpu/drm/rcar-du/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index 1f10fa0928b4..eb1e6d5cfed9 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -27,6 +27,6 @@ config DRM_RCAR_LVDS
 config DRM_RCAR_VSP
bool "R-Car DU VSP Compositor Support"
depends on DRM_RCAR_DU
-   depends on VIDEO_RENESAS_VSP1
+   depends on VIDEO_RENESAS_VSP1=y || (VIDEO_RENESAS_VSP1 && DRM_RCAR_DU=m)
help
  Enable support to expose the R-Car VSP Compositor as KMS planes.
-- 
2.7.0

Re: [PATCH][CFT]: dmaengine: make slave address physical

[Arnd Bergmann]

On Monday 22 February 2016 13:31:04 Wolfram Sang wrote:
> 
> > > Original-patch-by: Linus Walleij <linus.wall...@linaro.org>
> > 
> > You've dropped a few 
> > 
> > Original-patch-acked-by: Lee Jones <lee.jo...@linaro.org>
> > Original-patch-acked-by: Arnd Bergmann <a...@arndb.de>
> 
> I'd vote for dropping the "Original-patch-" prefix and keep the original
> SoB and Acks because the content of the patch is still the same. And
> while the new commit message is a lot more precise, it is also in the
> same spirit as the old one.
> 
> That being said:
> 
> Acked-by: Wolfram Sang <wsa+rene...@sang-engineering.com>
> Tested-by: Wolfram Sang <wsa+rene...@sang-engineering.com>
> 
> I tested it on my Lager board on top of my sdhi-uhs testing branch and
> used DMA with SD cards and for I2C transfers. No regressions seen. Also
> no build warnings.
> 

Acked-by: Arnd Bergmann <a...@arndb.de>

Re: [PATCH v2 1/5] dma-mapping: add dma_{map,unmap}_page_attrs

[Arnd Bergmann]

On Sunday 24 January 2016 22:43:57 Laurent Pinchart wrote:
> Hi Niklas,
> 
> (CC'ing LKML, linux-arch and Arnd Bergmann)
> 
> Thank you for the patch.
> 
> On Thursday 21 January 2016 15:01:31 Niklas Söderlund wrote:
> > Add a version of dmap_{map,unmap}_page that can pass on attributes to
> > the underlaying map_page. This already exists for dma_{map,unmap}_single
> > and dmap_{map,unmap}_sg versions.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> 

The patch looks fine, but won't apply any more now that the code has
been moved to include/linux/dma-mapping.h

Arnd

Re: [GIT PULL] Second Round of Renesas ARM Based SoC DT Updates for v4.7

[Arnd Bergmann]

On Thursday 28 April 2016 09:08:06 Simon Horman wrote:
> Second Round of Renesas ARM Based SoC DT Updates for v4.7
> 
> * Don't disable referenced optional clocks in DT of R-Car Gen 1 & 2 SoCs
> 

Pulled into next/dt, thanks!

Arnd

Re: [GIT PULL v2] Renesas ARM Based SoC DT Updates for v4.7

[Arnd Bergmann]

On Tuesday 26 April 2016 10:32:39 Geert Uytterhoeven wrote:
> Hi Arnd,
> 
> On Tue, Apr 26, 2016 at 10:12 AM, Arnd Bergmann <a...@arndb.de> wrote:
> > On Tuesday 26 April 2016 09:30:45 Geert Uytterhoeven wrote:
> >> Commit 93fafa81cad3da9c ("Merge branches 'next/dt' and 'next/dt64' into
> >> for-next") accidentally re-added the line
> >>
> >> clock-output-names = "can_clk";
> >>
> >> to arch/arm/boot/dts/r8a7791.dtsi, which was removed in commit
> >> f617604fe5d74b9c ("ARM: dts: r8a7791: Remove unnecessary
> >> clock-output-names properties") before.  It seems the same wrong
> >> conflict resolution was made before in commit ef3b08c6c0d85753 ("Merge
> >> branch 'fixes' into for-next").
> >>
> >> While this won't cause failures, can you please remove the offending
> >> line?
> >
> > I've merged renesas-fixes-for-v4.6 into next/dt now to resolve
> > the conflict there, otherwise Linus would have to do the same resolution
> > during the merge window.
> >
> > Thanks for letting us know. If you want to verify my new resolution,
> > please look at next/dt, which is the more important branch.
> 
> Thanks, but unfortunately your new merge resolution is incorrect, as it 
> revived
> the line
> 
> status = "disabled";
> 
> for the pcie_bus node.
> 

I've done the merge once more, noticed my mistake (vimdiff syntax
coloring was slightly misleading) and fixed it.

Arnd

Re: [GIT PULL] Second Round of Renesas ARM Based SoC R-Car SYSC Updates for v4.7

[Arnd Bergmann]

On Tuesday 26 April 2016 11:28:38 Simon Horman wrote:
> This pull request is based on a merge of:
> 
> * Thee previous round of such requests, tagged as renesas-rcar-sysc-for-v4.7,
>   which you have previously pulled.
> * The clk-renesas-for-v4.7-tag2 tag of Geert Uytterhoeven's renesas-drivers
>   tree which has been accepted into clk-next by Stephen Boyd.

I've pulled it into next/drivers now.

> There are follow-up DT changes that depend on this driver which I plan to
> send in a pull-request for in the near future.

Ok, I need to revisit your earlier explanation of the exact dependencies
here and see if we can find a way to merge this nicely. We usually
have the next/dt branch before the next/drivers branch, so this reverse
dependency would require a next/dt2 branch unless we can either change
the order for the 4.7 merge window for all platforms, or find a way
to merge (most of) the following DT changes without having the driver
done first.

Arnd

[PATCH] pinctrl: ish-pfc: avoid unused variable warning

[Arnd Bergmann]

After the conversion to devm_pinctrl_register, we get a warning in
sh_pfc_remove when CONFIG_PINCTRL_SH_PFC_GPIO is disabled:

drivers/pinctrl/sh-pfc/core.c: In function 'sh_pfc_remove':
drivers/pinctrl/sh-pfc/core.c:603:17: unused variable 'pfc' 
[-Werror=unused-variable]
  struct sh_pfc *pfc = platform_get_drvdata(pdev);

This moves the variable definition inside of the same ifdef
that has the only user, to get a clean build again.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
Fixes: 67ec8d7b4846 ("pinctrl: ish-pfc: Use devm_pinctrl_register() for pinctrl 
registration")
---
 drivers/pinctrl/sh-pfc/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index 7ed9c80ba3ed..c9ee31c9952b 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -600,9 +600,9 @@ static int sh_pfc_probe(struct platform_device *pdev)
 
 static int sh_pfc_remove(struct platform_device *pdev)
 {
+#ifdef CONFIG_PINCTRL_SH_PFC_GPIO
struct sh_pfc *pfc = platform_get_drvdata(pdev);
 
-#ifdef CONFIG_PINCTRL_SH_PFC_GPIO
sh_pfc_unregister_gpiochip(pfc);
 #endif
 
-- 
2.7.0

[PATCH] drm: rcar: remove unused variable

[Arnd Bergmann]

A cleanup for the rcar-du driver left an unused variable behind:

drm/rcar-du/rcar_du_drv.c: In function 'rcar_du_probe':
drm/rcar-du/rcar_du_drv.c:300:24: error: unused variable 'connector' 
[-Werror=unused-variable]

This removes the variable.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
Fixes: d63c25e4245a ("drm: rcar-du: Use generic drm_connector_register_all() 
helper")
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 0f251dc11082..fb9242d27883 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -297,7 +297,6 @@ static int rcar_du_probe(struct platform_device *pdev)
 {
struct device_node *np = pdev->dev.of_node;
struct rcar_du_device *rcdu;
-   struct drm_connector *connector;
struct drm_device *ddev;
struct resource *mem;
int ret;
-- 
2.7.0

Re: [GIT PULL] Second Round of Renesas ARM Based SoC Fixes for v4.6

[Arnd Bergmann]

On Thursday 28 April 2016 09:06:57 Simon Horman wrote:
> Second Round of Renesas ARM Based SoC Fixes for v4.6
> 
> * Don't disable referenced optional scif clock
> 

Pulled into fixes, thanks!

Arnd

Re: [GIT PULL] Renesas ARM Based SoC DT PM Domain Updates for v4.7

[Arnd Bergmann]

On Wednesday 27 April 2016 14:20:34 Simon Horman wrote:
> Please consider these Renesas ARM based SoC DT pm-domain updates for v4.7.
> 
> This pull requests is based on a merge of:
> 
> * "[GIT PULL] Second Round of Renesas ARM Based SoC R-Car SYSC Updates for
>   v4.7", tagged as renesas-rcar-sysc2-for-v4.7, which you have already
>   pulled.
> * "[GIT PULL v2] Renesas ARM Based SoC DT Updates for v4.7",
>   tagged as renesas-dt-for-v4.7, which you have also already pulled.
> 
> The reason for the somewhat tedious base on
> renesas-rcar-sysc2-for-v4.7, which provides driver changes,
> is a hard run-time dependency.
> 
> I also have a similar set of changes for arm64 which I will send separately.


Ok, I've started a next/late branch for this now, but unless something
else comes up that needs to be sent much later, I expect to submit
this as the last pull request at the same time as all the other ones
for 4.7.

For further clarification, am I right reading the above as meaning that
a 4.7 kernel is expected to still work with the dts files from 4.6,
but not the other way round?

Arnd

Re: [PATCHv2] i2c: rcar: add DMA support

[Arnd Bergmann]

On Saturday 14 May 2016 14:17:08 Niklas Söderlund wrote:
> +   chan = dma_request_slave_channel_reason(dev, chan_name);
> +   if (IS_ERR(chan)) {
> +   ret = PTR_ERR(chan);
> +   dev_dbg(dev, "request_channel failed for %s (%d)\n",
> +   chan_name, ret);
> +   return chan;
> +   }
> 

I think you should now use dma_request_chan() for new drivers.

Arnd

Re: [GIT PULL] Renesas ARM64 Based SoC DT PM Domain Updates for v4.7

[Arnd Bergmann]

On Friday 06 May 2016 09:21:25 Simon Horman wrote:
> Hi,
> 
> On Wed, Apr 27, 2016 at 02:20:10PM +1000, Simon Horman wrote:
> > Hi Olof, Hi Kevin, Hi Arnd,
> > 
> > Please consider these Renesas ARM64 based SoC DT PM Domain updates for v4.7.
> > 
> > This pull requests is based on a merge of:
> > 
> > * "[GIT PULL] Second Round of Renesas ARM Based SoC R-Car SYSC Updates for
> >   v4.7", tagged as renesas-rcar-sysc2-for-v4.7, which you have already
> >   pulled
> > * "[GIT PULL] Second Round of Renesas ARM64 Based SoC DT Updates for v4.7",
> >   tagged as renesas-arm64-dt2-for-v4.7, which I have sent a pull request
> >   for.
> > 
> > The reason for the somewhat tedious base on
> > renesas-rcar-sysc2-for-v4.7, which provides driver changes,
> > is a hard run-time dependency.
> > 
> > I also have a similar set of changes for ARM (32-bit) which I will send
> > separately.
> 
> I am wondering if this pull-request might have slipped through the cracks.
> 
> FWIW, all other pull requests I have submitted for v4.7 have been pulled
> (thanks!) including a similar pull-request for ARM (32-bit) which is
> present in the next/late branch of the arm-soc tree.
> 
> I do not plan to send any more non-fix pull requests for v4.7.
> And at this stage I am not aware of any fixes for v4.7.
> 

Indeed it did, thanks for the reminder! I've put it on my stack now
and should start doing merges later today when I'm through my mail.

Arnd

Re: [GIT PULL] Renesas ARM64 Based SoC DT PM Domain Updates for v4.7

[Arnd Bergmann]

On Wednesday 27 April 2016 14:20:10 Simon Horman wrote:
> Hi Olof, Hi Kevin, Hi Arnd,
> 
> Please consider these Renesas ARM64 based SoC DT PM Domain updates for v4.7.
> 
> This pull requests is based on a merge of:
> 
> * "[GIT PULL] Second Round of Renesas ARM Based SoC R-Car SYSC Updates for
>   v4.7", tagged as renesas-rcar-sysc2-for-v4.7, which you have already
>   pulled
> * "[GIT PULL] Second Round of Renesas ARM64 Based SoC DT Updates for v4.7",
>   tagged as renesas-arm64-dt2-for-v4.7, which I have sent a pull request
>   for.
> 
> The reason for the somewhat tedious base on
> renesas-rcar-sysc2-for-v4.7, which provides driver changes,
> is a hard run-time dependency.
> 
> I also have a similar set of changes for ARM (32-bit) which I will send
> separately.
> 

Pulled into next/late now along with the 32-bit branch. Thanks,

Arnd

Re: [GIT PULL v2] Second Round of Renesas ARM64 Based SoC DT Updates for v4.8

[Arnd Bergmann]

On Thursday, July 7, 2016 4:39:45 PM CEST Simon Horman wrote:
>   arm64: dts: r8a7796: Add SYSC PM Domains
> 

While doing some sanity checks on our branch contents, I noticed
that this commit breaks running "make dtbs" on top of our next/dt64
branch, and I'd like to avoid that.

I found a couple more problems (tegra and meson), and I ended up
fixing them all up (by merging the dependencies into next/dt64),
so everything is sorted for 4.8, but it would be good to do a
'make dtbs' run on a branch that has dt changes before you send it,
and I'll try to do the same when pulling it to avoid the problem in
the future.

Arnd

Re: [GIT PULL v2] Second Round of Renesas ARM64 Based SoC DT Updates for v4.8

[Arnd Bergmann]

On Thursday, July 7, 2016 4:39:45 PM CEST Simon Horman wrote:
> Second Round of Renesas ARM64 Based SoC DT Updates for v4.8
> 
> * Add support for  r8a7796/salvator-x (R-Car Gen 3 M3-W)
> * Add CAN support to r8a7795 (R-Car Gen 3 H3)
> 

Merged into next/late to better deal with the dependency, thanks!

Arnd

Re: [GIT PULL] Renesas ARM Based SoC DT Fixes for v4.8

[Arnd Bergmann]

On Friday, July 22, 2016 10:59:24 AM CEST Simon Horman wrote:
> On Thu, Jul 21, 2016 at 02:41:52PM +0200, Arnd Bergmann wrote:
> > On Monday, July 18, 2016 11:39:35 AM CEST Simon Horman wrote:
> > > Renesas ARM Based SoC DT Fixes for v4.8
> > > 
> > > * Corrections to r8a7792
> > > 
> > 
> > Merged into next/dt, thanks!
> 
> Thanks!
> 
> I have queued up another fix for v4.8 since sending the above to you.
> This time it is an SoC (C-code) rather than a DT fix. I am wondering
> if you could offer guidance on:
> 
> * If you would prefer me to split fixes out into separate (DT, SoC, ...)
>   branches? If so is that until around rc1 when everything has been
>   merged into the forthcoming release and thereafter you would prefer
>   a single fixes branch?

I prefer a separate fixes branch. It's fine for all other branches
to be based on top of this branch so you have a working baseline
for testing.

> * What pace you would like me to send fixes. I guess it depends on
>   where we are in the cycle. Currently I put the changes in next and
>   send what I have about once a week (or not if there are none :).
> 
> Typically there are very few fixes. But as I already have 3 for v4.8,
> including those in this pull request, I get the feeling a few more could
> emerge before v4.8 is released.

I'd say don't wait longer than a week before forwarding bugfixes.
If you find something urgent, just send that right away along with
whatever less urgent patches you have accumulated.

It's possible we won't merge it right away as we might all be busy
for a couple of days, but in the worst case that means you send
another fixes pull request that is a superset before we have merged
the first one.

Arnd

Re: Question about dma-ranges property for PCI host controllers

[Arnd Bergmann]

On Thursday, August 11, 2016 3:00:42 PM CEST Phil Edworthy wrote:
> Hi,
> 
> A few PCI host controllers use the "dma-ranges" property to specify the
> mapping from PCI bus addresses to physical addresses.
> 
> In the case of R-Car PCIe Host controllers, the intention was to set this
> property as a 1 to 1 mapping for all DDR that could be addressed by the
> device. However, there are some limitations for the R-Car controller which
> meant that we could only map a subset of the DDR range - this limitation
> has prompted us to work on enabling the IOMMU behind the PCI controller.
> 
> When there is an IOMMU behind the PCI controller, the "dma-ranges"
> property specifies the mapping from PCI bus addresses to an IOVA address.
> So should the property map all address space?
> 
> Note that this is not actually possible with the R-Car hardware, but I
> found that the IOVA address space is outside of the DDR address space
> that we were using so had change it.

It's a bit tricky: the dma-ranges properties are walked recursively,
and a PCI bus may be behind a few other bridges that each have a
nontrivial mapping, and the IOMMU may not be on the address space that
the PCI host sees.

In the past, we have said that the dma-ranges property should reflect
the address space that is used when programming the bridge registers
in the PCI host bridge itself.

I think we have also made the assumption that a PCI host bridge
with an IOMMU uses a flat 32-bit DMA address space that goes through
the IOMMU (possibly a separate address space per PCI function,
depending on the type of IOMMU).

One corner case that doesn't really fit in that model is a PCI host
bridge that requires the bridge register to be programmed in a special
way for the IOMMU to work (e.g. away from the RAM to the address that
is routed to the IOMMU).

Another tricky case is a PCI host that uses the IOMMU only for 32-bit
DMA masters but that does have a dma-ranges property that can be
used for direct mapping of all RAM through a nonzero offset that
gets set up according to dma-ranges.

Can you be more specific which of those cases you actually have here?

Arnd

Re: [GIT PULL] Renesas ARM Based SoC DT Fixes for v4.8

[Arnd Bergmann]

On Monday, July 18, 2016 11:39:35 AM CEST Simon Horman wrote:
> Renesas ARM Based SoC DT Fixes for v4.8
> 
> * Corrections to r8a7792
> 

Merged into next/dt, thanks!

Arnd

Re: next build: 143 builds: 3 failed, 140 passed, 4 errors, 131 warnings (next-20160715)

[Arnd Bergmann]

On Friday, July 15, 2016 5:37:02 AM CEST kernelci. org bot wrote:
> 
>   1  pm-rcar-gen2.c:(.init.text+0x740): undefined reference to 
> `platform_can_secondary_boot'

I sent a patch for it, Geert added an Ack, but it's still waiting to come
back through the renesas tree.

Note that the actual error is in arch/arm/mach-shmobile/platsmp.c,
not in pm-rcar-gen2.c

>   1  fs/fat/dir.c:758:424: internal compiler error: Segmentation fault

I reported the gcc but when it broke in 4.9, but it was only fixed in 6.1,
so this will have to wait for the kernelci infrastructure to do something
about it by updating their toolchain or blacklisting the config.

>   1  drivers/pinctrl/bcm/pinctrl-nsp-mux.c:356:20: error: 
> 'pinconf_generic_dt_node_to_map_group' undeclared here (not in a function)
>   1  drivers/pinctrl/bcm/pinctrl-cygnus-mux.c:739:20: error: 
> 'pinconf_generic_dt_node_to_map_group' undeclared here (not in a function)

I though I sent a patch for it but con't find it now. We basically need
this:

Subject: [PATCH] pinctrl: bcm: add OF dependencies

Signed-off-by: Arnd Bergmann <a...@arndb.de>

diff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig
index 7967c6723676..63246770bd74 100644
--- a/drivers/pinctrl/bcm/Kconfig
+++ b/drivers/pinctrl/bcm/Kconfig
@@ -60,6 +60,7 @@ config PINCTRL_IPROC_GPIO
 config PINCTRL_CYGNUS_MUX
bool "Broadcom Cygnus IOMUX driver"
depends on (ARCH_BCM_CYGNUS || COMPILE_TEST)
+   depends on OF
select PINMUX
select GENERIC_PINCONF
default ARCH_BCM_CYGNUS
@@ -103,6 +104,7 @@ config PINCTRL_NS2_MUX
 config PINCTRL_NSP_MUX
bool "Broadcom NSP IOMUX driver"
depends on (ARCH_BCM_NSP || COMPILE_TEST)
+   depends on OF
select PINMUX
select GENERIC_PINCONF
default ARCH_BCM_NSP


> Warnings summary:
> 
>  43  drivers/video/fbdev/core/fbmon.c:1497:67: warning: parameter 'specs' 
> set but not used [-Wunused-but-set-parameter]

I sent a patch on June 16 when this warning was only for "make W=1", but
never got a reply from the fbdev maintainers (which is not uncommon
for that subsystem). Now Andrew merged a patch to have the warning at the
default level, I'll send him my oneline patch so he can apply it as well:

--- a/drivers/video/fbdev/core/fbmon.c
+++ b/drivers/video/fbdev/core/fbmon.c
@@ -1496,7 +1496,6 @@ int fb_parse_edid(unsigned char *edid, struct 
fb_var_screeninfo *var)
 }
 void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
 {
-   specs = NULL;
 }
 void fb_edid_add_monspecs(unsigned char *edid, struct fb_monspecs *specs)
 {


>  14  drivers/i2c/i2c-core.c:269:20: warning: 'i2c_acpi_add_device' 
> defined but not used [-Wunused-function]

This was the result of an incomplete merge conflict resolution in linux-next,
hopefully to be resolved soon.

>   6  arch/arm64/configs/defconfig:352:warning: override: reassigning to 
> symbol PWM

I have to look into this.

>   4  WARNING: modpost: missing MODULE_LICENSE() in 
> drivers/dma/xilinx/zynqmp_dma.o

I have not seen this one before, but it should be obvious, Kedareswara rao 
Appana
just submitted the new driver recently and can probably send a fix.

>   3  drivers/misc/lkdtm_core.c:97:22: warning: 'jp_shrink_inactive_list' 
> defined but not used [-Wunused-function]
>   3  drivers/misc/lkdtm_core.c:89:13: warning: 'jp_ll_rw_block' defined 
> but not used [-Wunused-function]
>   3  drivers/misc/lkdtm_core.c:83:13: warning: 'jp_tasklet_action' 
> defined but not used [-Wunused-function]
>   3  drivers/misc/lkdtm_core.c:75:20: warning: 'jp_handle_irq_event' 
> defined but not used [-Wunused-function]
>   3  drivers/misc/lkdtm_core.c:68:21: warning: 'jp_do_irq' defined but 
> not used [-Wunused-function]
>   3  drivers/misc/lkdtm_core.c:340:16: warning: 'lkdtm_debugfs_entry' 
> defined but not used [-Wunused-function]
>   3  drivers/misc/lkdtm_core.c:114:12: warning: 'jp_scsi_dispatch_cmd' 
> defined but not used [-Wunused-function]
>   3  drivers/misc/lkdtm_core.c:106:12: warning: 'jp_hrtimer_start' 
> defined but not used [-Wunused-function]

This showed up today, and I sent a patch.

>   2  net/bluetooth/6lowpan.c:608:570: warning: 'addr_type' may be used 
> uninitialized in this function [-Wmaybe-uninitialized]

I assume this is the result of using an older compiler, I haven't looked at the
details. Olof's build bot doesn't have it.

>   2  lib/test_hash.c:234:7: warning: "HAVE_ARCH_HASH_64" is not defined 
> [-Wundef]
>   2  lib/test_hash.c:229:7: warning: "HAVE_ARCH_HASH_32" is not defined 
> [-Wundef]
>   2  lib/test_hash.c:224:7: warning: "HAVE_ARCH__HASH_32" is not defined 
> [-Wundef]
>   2  lib/test_hash.c:146:2: warning

Re: [PATCH/RFC v3 5/6] mmc: renesas_sdhi: add support for R-Car Gen3 SDHI DMAC

[Arnd Bergmann]

On Wednesday, July 6, 2016 10:23:29 PM CEST Simon Horman wrote:
> @@ -117,6 +124,23 @@ void sdhi_sys_dmac_init_dma(void);
>  static void sdhi_sys_dmac_init_dma(void) { }
>  #endif
>  
> +#if IS_ENABLED(CONFIG_MMC_SDHI_INTERNAL_DMA)
> +void sdhi_internal_dmac_init_dma(void);
> +#else
> +static void sdhi_internal_dmac_init_dma(void) { }
> +#endif
> +
> +static void sh_mobile_sdhi_init_dma(enum tmio_mmc_dmac_type dmac_type)
> +{
> +   switch (dmac_type) {
> +   case TMIO_MMC_INTERNAL_DMAC:
> +   return sdhi_internal_dmac_init_dma();
> +
> +   case TMIO_MMC_SYSC_DMAC:
> +   return sdhi_sys_dmac_init_dma();
> +   }
> +}
> +
>  static void sh_mobile_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
>  {
> u32 val;
> 

I've commented on this part for v2 already but got no reply. This
time I took a little more care to understand the structure of the
driver and how it gets modified.

The way I see it, we have the MMC_TMIO_CORE driver that consists of
tmio_mmc_pio.c and tmio_mmc_dma.c, which are tightly connected though the
second one is optional, and two front-ends named tmio_mmc.c and
sh_mobile_sdhi.c.

The first front-end never uses DMA, while the second one may or may
not use DMA but is always built with the DMA support linked into the
core driver.

I think abstracting the two DMA modes through a structure of
function pointers as you do is the right strategy, but to build on
top of that, we can change the link order:

- rename tmio_mmc_pio.c to tmio_mmc_core.c and make that the actual
  driver core (without DMA)
- tmio_mmc_dma.c becomes the main driver module for sh_mobile_sdhi
  and gets the sh_mobile_sdhi_driver structure, sh_mobile_sdhi_of_match
  table and module_platform_driver() statement
- the existing sh_mobile_sdhi.c is a library module that exports
  sh_mobile_sdhi_probe() and sh_mobile_sdhi_remove(), which now
  gain a 'struct tmio_mmc_dma_ops *' and a 'struct sh_mobile_sdhi_of_data *'
  argument and get called by the new probe function in what used to
  be tmio_mmc_dma.c.
- The new renesas_sdhi_internal_dmac.c becomes a third top-level
  module that also calls sh_mobile_sdhi_probe() but passes its
  own struct tmio_mmc_dma_ops and registers a different
  platform_driver that only matches the of_rcar_gen3_compatible.

Arnd

[PATCH] ARM: shmobile: don't call platform_can_secondary_boot on UP

[Arnd Bergmann]

For rcar-gen2, we build the SMP files even for UP configurations,
and that just broke:

arch/arm/mach-shmobile/built-in.o: In function `shmobile_smp_init_fallback_ops':
pm-rcar-gen2.c:(.init.text+0x40c): undefined reference to 
`platform_can_secondary_boot'

This adds an compile-time check before the call to platform_can_secondary_boot,
turning the function into an empty stub for UP configurations.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
Fixes: c21af444eace ("ARM: shmobile: smp: Add function to prioritize DT SMP")
---
 arch/arm/mach-shmobile/platsmp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-shmobile/platsmp.c b/arch/arm/mach-shmobile/platsmp.c
index f3dba6f356e2..02e21bceb085 100644
--- a/arch/arm/mach-shmobile/platsmp.c
+++ b/arch/arm/mach-shmobile/platsmp.c
@@ -40,5 +40,8 @@ bool shmobile_smp_cpu_can_disable(unsigned int cpu)
 bool __init shmobile_smp_init_fallback_ops(void)
 {
/* fallback on PSCI/smp_ops if no other DT based method is detected */
+   if (!IS_ENABLED(CONFIG_SMP))
+   return false;
+
return platform_can_secondary_boot() ? true : false;
 }
-- 
2.9.0

Re: [PATCH/RFC v2 5/5] mmc: renesas_sdhi: add support for R-Car Gen3 SDHI DMAC

[Arnd Bergmann]

On Thursday, June 30, 2016 12:05:40 AM CEST Simon Horman wrote:
> @@ -120,6 +127,26 @@ static int sdhi_sysc_dmac_init_dma(void)
>  }
>  #endif
>  
> +#if IS_ENABLED(CONFIG_MMC_SDHI_INTERNAL_DMA)
> +int sdhi_internal_dmac_init_dma(void);
> +#else
> +static int sdhi_internal_dmac_init_dma(void)
> +{
> +   return -EINVAL;
> +}
> +#endif
> +
> +static int sh_mobile_sdhi_init_dma(enum tmio_mmc_dmac_type dmac_type)
> +{
> +   switch (dmac_type) {
> +   case TMIO_MMC_INTERNAL_DMAC:
> +   return sdhi_internal_dmac_init_dma();
> +
> +   case TMIO_MMC_SYSC_DMAC:
> +   return sdhi_sysc_dmac_init_dma();
> +   }
> +}
> +

I think it would be nicer to turn the logic around and handle the
different kinds of TMIO hardware like we do it for the sdhci types:

Make the common portion of the driver a module that just exports
functions but doesn't register a driver, and then put each variant
(PIO, internal DMA, SYSC-DMA) into a separate module that registers
its own driver.

Do you think that makes sense in this case?

Arnd

Re: [PATCH 1/2] ARM: add ARM_SINGLE_ARMV7 as config option

[Arnd Bergmann]

On Thursday, February 9, 2017 8:21:43 PM CET Chris Brandt wrote:
> On Thursday, February 09, 2017, Florian Fainelli worte:
> > > I think the closest I might have come was to purposely break the build
> > > if more then 1 was select, but that still didn't stop you from making
> > > the selection.
> > >
> > > If someone smarter than me has a way to do (not just an idea...I
> > received
> > > lots of ideas but none of them worked), I'd be happy to hear it.
> > 
> > I am definitively not a Kbuild expert, but it would almost necessarily
> > require introduce some kind of new type in the Kconfig/Kbuild syntax
> > that does something like that:
> > 
> > - have a way to count the number of symbols that are selected and do a
> > "if ARCH_MULTI_V6_V7" (or an arbitrary expression) this most likely
> > should exist internally within Kconfig
> > 
> > - introduce a new type of Kconfig type which is a "count", and gets
> > assigned this value that we just counted, something that could look like
> > this:
> > 
> > count ARCH_MULTI_V6_V7_COUNT
> >   tracks ARCH_MULTI_V6_V7
> 
> I did try the counting thing, but couldn't get it to work.
> I admit though, I did stop when the next step was to create a new type
> kind of thing that I could use for counting. That seemed to start going
> down a deeper path than I was hoping for.

I also couldn't come up with something working when I looked at this,
and it wouldn't solve the related problem of platforms that we want to
be able to use with or without MMU: You can't make the decision of
whether allow an MMU based on the number of platforms since most
platform options can only be offered depending on the setting of
CONFIG_MMU.

> However,
> I am hesitant to go and try anything else because everything I've submitted
> so far has been NACKed. The only thing Russell said he'd agree to is a top
> level single-platform option. But, since that all got taken out, I assume
> there's some resistance to putting it back in.

And I really don't like adding new top-level for a platform here, it
brings us back to the same problems we had before we moved most platforms
to ARCH_MULTIPLATFORM, and it doesn't solve the remaining problems we still
have:

- In some platforms, the decision would have to be done on a per-board
  level, as each board can have its memory at a different location
  base on which chipselect line got connected to the RAM and NOR flash
  respectively

- Some (few) platforms actually have separate top-level Kconfig options
  but are actually very closely related and you could have a kernel
  for all of them even with !MMU and XIP_KERNEL. The most important
  one here is ARM Versatile/Realview/Integrator/Vexpress that have
  more in common than things we put behind a common Kconfig option in
  other platforms.

- CONFIG_DEBUG_UNCOMPRESS has a very similar requirements to
  XIP_KERNEL and !MMU, and we currently allow it for any machine,
  with a lot of flexibility in configuring that always breaks
  running on any machine other than the one you are targetting.

Arnd

Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

[Arnd Bergmann]

On Thursday, December 29, 2016 11:45:03 PM CET Nikita Yushchenko wrote:
> 
>  static int __swiotlb_dma_supported(struct device *hwdev, u64 mask)
>  {
> +#ifdef CONFIG_PCI
> +   if (dev_is_pci(hwdev)) {
> +   struct pci_dev *pdev = to_pci_dev(hwdev);
> +   struct pci_host_bridge *br = pci_find_host_bridge(pdev->bus);
> +
> +   if (br->dev.dma_mask && (*br->dev.dma_mask) &&
> +   (mask & (*br->dev.dma_mask)) != mask)
> +   return 0;
> +   }
> +#endif
> if (swiotlb)
> return swiotlb_dma_supported(hwdev, mask);
> return 1;
> 

I think it's wrong to make this a special case for PCI.

Instead, we should follow the dma-ranges properties during dma_set_mask()
to ensure we don't set a mask that any of the parents up to the root
cannot support.

Arnd

Re: [PATCH v4 0/3] ARM: l2c: add l2c support for RZ/A1

[Arnd Bergmann]

On Thu, Feb 16, 2017 at 5:44 PM, Russell King - ARM Linux
<li...@armlinux.org.uk> wrote:
> On Thu, Feb 16, 2017 at 11:17:39AM -0500, Chris Brandt wrote:
>> The PL310 in the Renesas RZ/A1 SoC (R7S72100) does not have the sideband
>> signals connected between the CPU and L2C. According the PL310 TRM,
>> sideband signals are optional.
>>
>> If a PL310 is added to a system, but the sideband signals are not
>> connected, some Cortex A9 optimizations cannot be used. In particular,
>> enabling Full Line Zeros in the CA9 without sidebands connected will
>> crash the system since the CA9 will expect the L2C to perform operations,
>> yet the L2C never gets the commands.
>>
>> This series adds the option to not enable anything in the PL310 that
>> uses sidebands, and then adds L2C support to the RZ/A1 DT.
>>
>> v4:
>> * changed l2x0_bresp_dis to l2x0_bresp_disable
>> * changed l2x0_flz_dis to l2x0_flz_disable
>
> Looks good, thanks.
>
> I'll want to merge patch 1, but I suspect the other two patches need
> to go through arm-soc, which will cause a problem due to being merged
> independently...  Arnd?

I just checked what we have queued up in next/dt for this platform,
and I'm fairly sure that there are no conflicts, so you can pick up all three
and add

Acked-by: Arnd Bergmann <a...@arndb.de>

 Arnd

Re: [GIT PULL] Renesas ARM Based SoC Fixes for v4.8

[Arnd Bergmann]

On Wednesday, September 7, 2016 9:41:04 AM CEST Simon Horman wrote:
> Renesas ARM Based SoC Fixes for v4.8
> 
> * Correct R-Car Gen2 regulator quirk
> 
Pulled into fixes, thanks!

Arnd

Re: [GIT PULL] Second Round of Renesas ARM Based SoC DT Updates for v4.9

[Arnd Bergmann]

On Wednesday, September 7, 2016 9:49:30 AM CEST Simon Horman wrote:
> Second Round of Renesas ARM Based SoC DT Updates for v4.9
> 
> Fixes (for v4.9):
> * Correct PWM clock parent on r8a7794 SoC
> 
> Clean-up:
> * Remove obsolete vsp1 properties from r8a779[01] SoCs
> 
> New boards:
> * Add r8a7792/wheat and r7s72100/rskrza1 boards
> 
> Enablement:
> * Enable LEDs, DU, SDHI on r8a7792/blanche board
> * Enable MMCIF and SDHI on r8a7794/alt board
> * Add SPI and VSP1 to r8a7792 SoC
> * Add ethernet to r7s72100 SoC
> 

Pulled into next/dt, thanks!

Arnd

Re: [GIT PULL] Renesas ARM64 Based SoC DT Updates for v4.9

[Arnd Bergmann]

On Thursday, September 8, 2016 9:43:24 AM CEST Simon Horman wrote:
> Hi Olof, Hi Kevin, Hi Arnd,
> 
> Please consider these Renesas ARM64 based SoC DT updates for v4.9.
> 
> This pull request is based on the sh-pfc-for-v4.9-tag2 of
> Geert Uytterhoeven's renesas-driver's tree which is included in the
> devel and for-next branches of Linus Walleij's linux-pinctrl tree.
> 
> 

Pulled into next/late because of the dependency.

We'll send it during the merge window after the dependencies
are all merged upstream. Thanks,

Arnd

Re: [GIT PULL] Renesas ARM64 Based SoC Defconfig Updates for v4.9

[Arnd Bergmann]

On Wednesday, September 7, 2016 9:41:26 AM CEST Simon Horman wrote:
> Renesas ARM64 Based SoC Defconfig Updates for v4.9
> 
> * Enable HSUSB and SDHI
> 

Pulled into next/arm64, thanks!

Arnd

Re: [GIT PULL] Renesas ARM Based SoC DT Updates for v4.9

[Arnd Bergmann]

On Monday, August 15, 2016 10:55:35 AM CEST Simon Horman wrote:
> Renesas ARM Based SoC DT Updates for v4.9
> 
> * Add DU, VIN, I2C, SDHI, EtherAVB, GPIO support to r8a7792
> * Enable CAN0 on r8a7792/blanche
> * Enable sound on r8a7794/silk
> * Correct SDHI register size on r8a7794
> 

Pulled into next/dt, thanks and sorry for the long delay.

Arnd

Re: [GIT PULL] Renesas ARM Based SoC Updates for v4.9

[Arnd Bergmann]

On Wednesday, September 7, 2016 9:41:38 AM CEST Simon Horman wrote:
> Renesas ARM Based SoC Updates for v4.9
> 
> Clean-up:
> * Only use smp_init when SMP is selected
> 
> Enablement:
> * Add debug-ll support for r8a7992
> 

Pulled into next/soc, thanks!

Arnd

Re: [PATCHv9 0/6] dmaengine: rcar-dmac: add iommu support for slave transfers

[Arnd Bergmann]

On Thursday, September 15, 2016 9:56:51 PM CEST Vinod Koul wrote:
> On Wed, Aug 10, 2016 at 11:07:10PM +0530, Vinod Koul wrote:
> > On Wed, Aug 10, 2016 at 01:22:13PM +0200, Niklas Söderlund wrote:
> > > Hi,
> > > 
> > > This series tries to solve the problem with DMA with device registers
> > > (MMIO registers) that are behind an IOMMU for the rcar-dmac driver. A
> > > recent patch '9575632 (dmaengine: make slave address physical)'
> > > clarifies that DMA slave address provided by clients is the physical
> > > address. This puts the task of mapping the DMA slave address from a
> > > phys_addr_t to a dma_addr_t on the DMA engine.
> > > 
> > > Without an IOMMU this is easy since the phys_addr_t and dma_addr_t are
> > > the same and no special care is needed. However if you have a IOMMU you
> > > need to map the DMA slave phys_addr_t to a dma_addr_t using something
> > > like this.
> > > 
> > > This series is based on top of v4.8-rc1. And I'm hoping to be able to 
> > > collect a
> > > Ack from Russell King on patch 4/6 that adds the ARM specific part and 
> > > then be
> > > able to take the whole series through the dmaengine tree. If this is not 
> > > the
> > > best route I'm more then happy to do it another way.
> > > 
> > > It's tested on a Koelsch with CONFIG_IPMMU_VMSA and by enabling the
> > > ipmmu_ds node in r8a7791.dtsi. I verified operation by interacting with
> > > /dev/mmcblk1, i2c and the serial console which are devices behind the
> > > iommu.
> > 
> > As I said in last one, the dmaengine parts look fine to me. But to go thru
> > dmaengine tree I would need ACK on non dmaengine patches.
> 
> I havent heard back from this one and I am inclined to merge this one now.
> If anyone has any objects, please speak up now...
> 
> Also ACKs welcome...
> 


I had not looked at the series earlier, but this version looks entirely
reasonable to me, so

Acked-by: Arnd Bergmann <a...@arndb.de>


One concern I have is that we might get an awkward situation if we
ever encounter one DMA engine hardware that is used in different
systems that all have an IOMMU, but on some of them the connection
between the DMA master and the slave FIFO bypasses the IOMMU
while on others the IOMMU is required. I don't have any idea for
how this could be handled in a generic way, so my best answer
here is to hope we never get there, and if we do, handle it
using some local hack in the driver.

Arnd

Re: [PATCH/RFC 4/4] soc: renesas: Identify SoC and register with the SoC bus

[Arnd Bergmann]

On Tuesday, October 4, 2016 11:09:27 AM CEST Geert Uytterhoeven wrote:
> Identify the SoC type and revision, and register this information with
> the SoC bus, so it is available under /sys/devices/soc0/, and can be
> checked where needed using soc_device_match().
> 
> In addition, on SoCs that support it, the product ID is read from a
> hardware register and validated, to catch accidental use of a DTB for a
> different SoC.
> 
> Example:
> 
> Detected Renesas r8a7791 ES1.0
> ...
> # cat /sys/devices/soc0/{family,machine,soc_id,revision}
> R-Car Gen2
> Koelsch
> r8a7791
> ES1.0
> 

Seems all reasonable.

> Signed-off-by: Geert Uytterhoeven 
> ---
> This patch does NOT add a call to
> 
> of_platform_default_populate(NULL, NULL,
>  soc_device_to_device(soc_dev));
> 
> Contrary to suggested by commit 74d1d82cdaaec727 ("drivers/base: add bus
> for System-on-Chip devices), doing so would not only move on-SoC devices
> from /sys/devices/platform/ to /sys/devices/soc0/, but also all other
> board (off-SoC) devices specified in the DTB.

Right, we have moved away from that a while ago, and now just
use the device for identification, not to model the device
hierarchy.

> diff --git a/drivers/soc/renesas/renesas-soc.c 
> b/drivers/soc/renesas/renesas-soc.c
> new file mode 100644
> index ..74b72e4112b8889e
> --- /dev/null
> +++ b/drivers/soc/renesas/renesas-soc.c
> @@ -0,0 +1,266 @@
> +/*
> + * Renesas SoC Identification
> + *
> + * Copyright (C) 2014-2016 Glider bvba
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +
> +struct renesas_family {
> + const char name[16];
> + u32 reg;/* CCCR, PVR, or PRR */
> +};
> +
> +static const struct renesas_family fam_emev2 __initconst = {
> + .name   = "Emma Mobile EV2",
> +};

As this is not related to the others and doesn't have the respective
register, I'd leave the platform out of this, and possibly have
a separate driver for it.

> +static const struct renesas_family fam_rza __initconst = {
> + .name   = "RZ/A",
> +};

I'm not sure about the relationship between this one and the others,
maybe it should be treated in the same way as emev2 and left out from
this driver?

> +static const struct renesas_family fam_rmobile __initconst = {
> + .name   = "R-Mobile",
> + .reg= 0xe600101c,   /* CCCR (Common Chip Code Register) */
> +};
> +
> +static const struct renesas_family fam_rcar_gen1 __initconst = {
> + .name   = "R-Car Gen1",
> + .reg= 0xff44,   /* PRR (Product Register) */
> +};
> +
> +static const struct renesas_family fam_rcar_gen2 __initconst = {
> + .name   = "R-Car Gen2",
> + .reg= 0xff44,   /* PRR (Product Register) */
> +};
> +
> +static const struct renesas_family fam_rcar_gen3 __initconst = {
> + .name   = "R-Car Gen3",
> + .reg= 0xfff00044,   /* PRR (Product Register) */
> +};
> +
> +static const struct renesas_family fam_rzg __initconst = {
> + .name   = "RZ/G",
> + .reg= 0xff44,   /* PRR (Product Register) */
> +};
> +
> +static const struct renesas_family fam_shmobile __initconst = {
> + .name   = "SH-Mobile",
> + .reg= 0xe600101c,   /* CCCR (Common Chip Code Register) */
> +};

These seem to fall into two distinct categories, maybe there is a
better way to group them. What device contain the two kinds of
registers (PRR, CCCR)?

Hardcoding the register address seems rather ugly here, so maybe
there is a way to have two separate probe methods based on the
surrounding register range, and then bind to that?

> +static const struct of_device_id renesas_socs[] __initconst = {
> +#ifdef CONFIG_ARCH_EMEV2
> + { .compatible = "renesas,emev2",.data = _emev2 },
> +#endif
> +#ifdef CONFIG_ARCH_R7S72100
> + { .compatible = "renesas,r7s72100", .data = _rz_a1h },
> +#endif
> +#ifdef CONFIG_ARCH_R8A73A4

I think the #ifdefs here will result in warnings for unused symbols 
when the Kconfig symbols are disabled.

Arnd

Re: [PATCHv9 0/6] dmaengine: rcar-dmac: add iommu support for slave transfers

[Arnd Bergmann]

On Friday, September 16, 2016 12:48:23 PM CEST Laurent Pinchart wrote:
> On Friday 16 Sep 2016 11:07:48 Arnd Bergmann wrote:
> > On Thursday, September 15, 2016 9:56:51 PM CEST Vinod Koul wrote:
> > > On Wed, Aug 10, 2016 at 11:07:10PM +0530, Vinod Koul wrote:
> > >> On Wed, Aug 10, 2016 at 01:22:13PM +0200, Niklas Söderlund wrote:
> > I had not looked at the series earlier, but this version looks entirely
> > reasonable to me, so
> > 
> > Acked-by: Arnd Bergmann <a...@arndb.de>
> > 
> > 
> > One concern I have is that we might get an awkward situation if we ever
> > encounter one DMA engine hardware that is used in different systems that all
> > have an IOMMU, but on some of them the connection between the DMA master and
> > the slave FIFO bypasses the IOMMU while on others the IOMMU is required.
> 
> Do you mean systems where some of the channels of a specific DMA engine go 
> through the IOMMU while others do not ? We indeed have no solution today for 
> such a situation.

I wasn't thinking quite that far, though that is also a theoretical
problem. However, the simple solution would be to have a bit in the DMA
specifier let the driver know whether translation is needed or not.

The simpler case I was thinking of is where the entire DMA engine
either goes through an IOMMU or doesn't (depending on the integration
into the SoC), so we'd have to find out through some DT property
or compatible string in the DMA enginen driver.

> The problem is a bit broader than that, we'll also have an issue with DMA 
> engines that have different channels served by different IOMMUs.

Do you mean a theoretical problem, or a chip that you already know exists?

> I recall 
> discussing this in the past with you, and the solution you proposed was to 
> add 
> a channel index to struct dma_attrs seems good to me. To support the case 
> where some channels don't go through an IOMMU we would only need support for 
> null entries in the IOMMUs list associated with a device (for instance in the 
> DT case null entries in the iommus property).
> 
> Now I see that struct dma_attrs has been replaced by unsigned long in
> 
> commit 00085f1efa387a8ce100e3734920f7639c80caa3
> Author: Krzysztof Kozlowski <k.kozlow...@samsung.com>
> Date:   Wed Aug 3 13:46:00 2016 -0700
> 
> dma-mapping: use unsigned long for dma_attrs
> 
> We still have enough bits to reserve some of them for a channel number, but 
> I'm not very happy with that patch as I can see how a future proposal to 
> handle the channel number through the DMA attributes will get rejected on the 
> grounds of bits starvation then :-(

Agreed, that can become interesting.

Arnd

Re: [PATCHv9 0/6] dmaengine: rcar-dmac: add iommu support for slave transfers

[Arnd Bergmann]

On Friday, September 16, 2016 3:09:29 PM CEST Laurent Pinchart wrote:
> > I wasn't thinking quite that far, though that is also a theoretical
> > problem. However, the simple solution would be to have a bit in the DMA
> > specifier let the driver know whether translation is needed or not.
> > 
> > The simpler case I was thinking of is where the entire DMA engine
> > either goes through an IOMMU or doesn't (depending on the integration
> > into the SoC), so we'd have to find out through some DT property
> > or compatible string in the DMA enginen driver.
> 
> Don't we already get that information from the iommus DT property ? If the 
> DMA 
> engine goes through an IOMMU the property will be set, otherwise it will not.

It depends. A dmaengine typically at least has two DMA masters,
possibly more. It's likely that some dmaengine implementations are
connected to RAM through an IOMMU, but have direct access to an
I/O bus for the slave FIFOs.

> > > The problem is a bit broader than that, we'll also have an issue with DMA
> > > engines that have different channels served by different IOMMUs.
> > 
> > Do you mean a theoretical problem, or a chip that you already know exists?
> 
> That's theoretical. The problem I'm facing today is a DMA engine whose 
> channels are served by different ports of the same IOMMU. This works in a 
> suboptimal way because I have to keep all the IOMMU ports enabled regardless 
> of whether they're used or not, as the DMA engine and IOMMU APIs don't carry 
> channel information.

Ok

Arnd

Re: [PATCH v4 0/2] ARM: cleanup PCI specific configs

[Arnd Bergmann]

On Wednesday, September 14, 2016 3:49:04 PM CEST Kishon Vijay Abraham I wrote:
> This series was initially sent to add support for two PCIe
> ports in dra7. This included selecting PCI_DOMAINS config
> in SOC_DRA7XX.
> 
> However from the review, PCI_DOMAINS can instead be selected
> from ARCH_MULTIPLATFORM. This is fixed in this series along
> with removing PCI_DOMAINS from other configs.
> 
> Changes from v3:
> *) Added *Acked-by:*
> *) Fixed $subject to not have *Fix*
> 
> Kishon Vijay Abraham I (2):
>   ARM: stop *MIGHT_HAVE_PCI* config from being selected redundantly
>   ARM: select PCI_DOMAINS config from ARCH_MULTIPLATFORM
> 

Applied both to next/cleanup now.

Arnd

Re: Question about dma-ranges property for PCI host controllers

[Arnd Bergmann]

On Friday, August 12, 2016 9:43:44 AM CEST Phil Edworthy wrote:
> On 11 August 2016 16:09 Arnd Bergmann wrote:

> > In the past, we have said that the dma-ranges property should reflect
> > the address space that is used when programming the bridge registers
> > in the PCI host bridge itself.
> > 
> > I think we have also made the assumption that a PCI host bridge
> > with an IOMMU uses a flat 32-bit DMA address space that goes through
> > the IOMMU (possibly a separate address space per PCI function,
> > depending on the type of IOMMU).
> I saw Robin Murphy's patches for PCI IOMMU map bindings, though at the
> moment to get things going I'm ignoring it because it will require quite a lot
> of changes to the iommu/ipmmu-vmsa driver. Other IOMMU drivers will
> also have to change a fair bit to support this new binding.

Ok

> > One corner case that doesn't really fit in that model is a PCI host
> > bridge that requires the bridge register to be programmed in a special
> > way for the IOMMU to work (e.g. away from the RAM to the address that
> > is routed to the IOMMU).
> In our case, there is nothing special in programming the bridge registers
> for use with an IOMMU other than the range of addresses that is exposed.
> The PCI host has a HW limitation that the AXI bus addresses must be
> 32-bit. The HW will allow you to set up the bridge registers so that PCI
> bus addresses above 32-bits are mapped into the 32-bit AXI space. This
> isn't used though at the moment, the PCI:AXI mapping is simply 1:1.
> 
> Simply changing the dma-ranges prop to specify all of the 32-bit range is
> enough to get it to work with the IOMMU.

That is the problem I was referring to above: the dma-ranges property
contents depend on whether the IOMMU is used or not, and that
is not something you know in the DT, because it depends on the OS.

> Without IOMMU, the dma-ranges prop was:
> dma-ranges = <0x4200 0 0x4000 0 0x4000 0 0x4000>;

Ok, so you map the second 1GB of PCI space into the second 1GB of AXI
space, which presumably is also the first 1GB of the physical RAM
at CPU address 0x, correct?

> Note that this does not cover all of DDR, as there is 3GiB above the 32-bit
> address space. Other restrictions in the way the bridge registers are
> programmed mean we cannot even map all DDR in the 32-bit space.
> 
> With the IOMMU, the dma-ranges prop is simply:
> dma-ranges = <0x4200 0 0x 0 0x 1 0x>;

Does this mean the IOMMU virtual address space covers the entire 4GB
of low addresses, or is it a subset of that space, e.g. the first
1GB, below where the PCI bus sees the RAM?

> > Another tricky case is a PCI host that uses the IOMMU only for 32-bit
> > DMA masters but that does have a dma-ranges property that can be
> > used for direct mapping of all RAM through a nonzero offset that
> > gets set up according to dma-ranges.
> I don't think that applies, though I'm struggling a bit to understand your
> comment.

It doesn't apply since the 32-bit limitation you see is part of the
PCI host bridge, not a limitation of some PCI devices.

Arnd

[PATCH] ravb: avoid unused function warnings

[Arnd Bergmann]

When CONFIG_PM_SLEEP is disabled, we get a couple of harmless warnings:

drivers/net/ethernet/renesas/ravb_main.c:2117:12: error: 'ravb_resume' defined 
but not used [-Werror=unused-function]
drivers/net/ethernet/renesas/ravb_main.c:2104:12: error: 'ravb_suspend' defined 
but not used [-Werror=unused-function]

The simplest solution here is to replace the #ifdef with __maybe_unused
annotations, which lets the compiler do the right thing by itself.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
Fixes: 0184165b2f42 ("ravb: add sleep PM suspend/resume support")
---
 drivers/net/ethernet/renesas/ravb_main.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c 
b/drivers/net/ethernet/renesas/ravb_main.c
index cad23ba06904..630536bc72f9 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2100,8 +2100,7 @@ static int ravb_remove(struct platform_device *pdev)
return 0;
 }
 
-#ifdef CONFIG_PM
-static int ravb_suspend(struct device *dev)
+static int __maybe_unused ravb_suspend(struct device *dev)
 {
struct net_device *ndev = dev_get_drvdata(dev);
int ret = 0;
@@ -2114,7 +2113,7 @@ static int ravb_suspend(struct device *dev)
return ret;
 }
 
-static int ravb_resume(struct device *dev)
+static int __maybe_unused ravb_resume(struct device *dev)
 {
struct net_device *ndev = dev_get_drvdata(dev);
struct ravb_private *priv = netdev_priv(ndev);
@@ -2149,7 +2148,7 @@ static int ravb_resume(struct device *dev)
return ret;
 }
 
-static int ravb_runtime_nop(struct device *dev)
+static int __maybe_unused ravb_runtime_nop(struct device *dev)
 {
/* Runtime PM callback shared between ->runtime_suspend()
 * and ->runtime_resume(). Simply returns success.
@@ -2166,17 +2165,12 @@ static const struct dev_pm_ops ravb_dev_pm_ops = {
SET_RUNTIME_PM_OPS(ravb_runtime_nop, ravb_runtime_nop, NULL)
 };
 
-#define RAVB_PM_OPS (_dev_pm_ops)
-#else
-#define RAVB_PM_OPS NULL
-#endif
-
 static struct platform_driver ravb_driver = {
.probe  = ravb_probe,
.remove = ravb_remove,
.driver = {
.name   = "ravb",
-   .pm = RAVB_PM_OPS,
+   .pm = _dev_pm_ops,
.of_match_table = ravb_match_table,
},
 };
-- 
2.9.0

Re: [PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus

[Arnd Bergmann]

On Monday, October 31, 2016 12:30:55 PM CET Geert Uytterhoeven wrote:

> v2:
>   - Drop SoC families and family names; use fixed "Renesas" instead,

I think I'd rather have seen the family names left in there, but it's
not important, so up to you.

>   - Use "renesas,prr" and "renesas,cccr" device nodes in DT if
> available, else fall back to hardcoded addresses for compatibility
> with existing DTBs,

I only see patches 2, 3, 5, and 7 in my inbox, so I'm lacking the DT
binding for these, among other things.

It does seem wrong to have a device node for a specific register though.
Shouldn't the node be for the block of registers that these are inside
of?

>   - Don't register the SoC bus if the chip ID register is missing,

Why? My objection was to hardcoding the register, not to registering
the device? I think I'd rather see the device registered with an
empty revision string.

> +#define CCCR   0xe600101c  /* Common Chip Code Register */
> +#define PRR0xff44  /* Product Register */
> +#define PRR3   0xfff00044  /* Product Register on R-Car Gen3 */
> +
> +static const struct of_device_id renesas_socs[] __initconst = {
> +#ifdef CONFIG_ARCH_R8A73A4
> +   { .compatible = "renesas,r8a73a4",  .data = (void *)PRR, },
> +#endif
> +#ifdef CONFIG_ARCH_R8A7740
> +   { .compatible = "renesas,r8a7740",  .data = (void *)CCCR, },
> +#endif

My preference here would be to list the register address only for
SoCs that are known to need them, while also having .dtb files that
don't have the nodes.

> +static int __init renesas_soc_init(void)
> +{
> +   struct soc_device_attribute *soc_dev_attr;
> +   const struct of_device_id *match;
> +   void __iomem *chipid = NULL;
> +   struct soc_device *soc_dev;
> +   struct device_node *np;
> +   unsigned int product;
> +
> +   np = of_find_matching_node_and_match(NULL, renesas_socs, );
> +   if (!np)
> +   return -ENODEV;
> +
> +   of_node_put(np);
> +
> +   /* Try PRR first, then CCCR, then hardcoded fallback */
> +   np = of_find_compatible_node(NULL, NULL, "renesas,prr");
> +   if (!np)
> +   np = of_find_compatible_node(NULL, NULL, "renesas,cccr");
> +   if (np) {
> +   chipid = of_iomap(np, 0);
> +   of_node_put(np);
> +   } else if (match->data) {
> +   chipid = ioremap((uintptr_t)match->data, 4);
> +   }
> +   if (!chipid)
> 

Here, I'd turn the order around and look for the DT nodes of the
devices first. Only if they are not found, look at the compatible
string of the root node. No need to search for a node though,
you know which one it is when you look for a compatible =
"renesas,r8a73a4".

Arnd

Re: [PATCH v2 0/7] soc: renesas: Identify SoC and register with the SoC bus

[Arnd Bergmann]

On Wednesday, November 9, 2016 2:34:33 PM CET Geert Uytterhoeven wrote:
> >
> > And Samsung.
> > Shall I create the immutable branch now?
> 
> Arnd: are you happy with the new patches and changes?

I still had some comments for patch 7, but that shouldn't stop
you from creating a branch for the first three so everyone can
build on top of that.

Arnd

Re: [PATCH v2 0/7] soc: renesas: Identify SoC and register with the SoC bus

[Arnd Bergmann]

On Wednesday, November 9, 2016 6:19:06 PM CET Geert Uytterhoeven wrote:
> On Wed, Nov 9, 2016 at 5:56 PM, Arnd Bergmann <a...@arndb.de> wrote:
> > On Wednesday, November 9, 2016 2:34:33 PM CET Geert Uytterhoeven wrote:
> >> > And Samsung.
> >> > Shall I create the immutable branch now?
> >>
> >> Arnd: are you happy with the new patches and changes?
> >
> > I still had some comments for patch 7, but that shouldn't stop
> > you from creating a branch for the first three so everyone can
> > build on top of that.
> 
> Thanks!
> 
> What about patch [4/7]?
> Haven't you received it? Your address was in the To-line for all 7 patches.

Ok, I see it now, looks good. That should be included as well then.

Arnd

Re: [PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus

[Arnd Bergmann]

On Thursday, November 10, 2016 11:19:20 AM CET Geert Uytterhoeven wrote:
> Hi Arnd,
> 
> Thanks for your comments!
> 
> On Wed, Nov 9, 2016 at 5:55 PM, Arnd Bergmann <a...@arndb.de> wrote:
> > On Monday, October 31, 2016 12:30:55 PM CET Geert Uytterhoeven wrote:
> >> v2:
> >>   - Drop SoC families and family names; use fixed "Renesas" instead,
> >
> > I think I'd rather have seen the family names left in there, but it's
> > not important, so up to you.
> 
> They're not useful for matching, as family names may change anytime, and don't
> always say much about the hardware capabilities.
> E.g. SH-Mobile -> R-Mobile -> R-Car | RZ/A | RZ/G
> Some SH-Mobile (even some R-Car) parts are SuperH only, others have ARM and
> SuperH.
> 
> At least the SoC part numbers are stable (hmm, sh73a0 == r8a73a0).

I think the marketing names are much more useful for humans looking
at the sysfs files than the kernel doing matching on, but both use
cases are important.

> >>   - Use "renesas,prr" and "renesas,cccr" device nodes in DT if
> >> available, else fall back to hardcoded addresses for compatibility
> >> with existing DTBs,
> >
> > I only see patches 2, 3, 5, and 7 in my inbox, so I'm lacking the DT
> > binding for these, among other things.
> 
> I understand you've received them in the mean time?

Yes, I found them in my inbox later, not sure why I didn't see them
at first.

> > It does seem wrong to have a device node for a specific register though.
> > Shouldn't the node be for the block of registers that these are inside
> > of?
> 
> On R-Mobile APE6, R-Car Gen2 and Gen3, PRR is a lone register.
> On R-Car Gen1, it's not even documented (and doesn't exist on all parts).

It just seems odd to have it at address 0xff44 when all the other
devices are at page-aligned addresses. Do you mean that accessing
0xff40 or 0xff48 will result in a bus-level exception for a
missing register and just 0xff44 is actually valid for access,
or is it just the only thing that is documented?

> On SH-Mobile/R-Mobile, CCCR may be part of the HPB/APB register block, which
> we further don't touch at all.
> On R-Car Gen2, it's not documented, but does exist.

This is where the family names would come in handy ;-) I now have
no idea which chip(s) you are referring to.

If you know the name of the register block, just put it into DT with
that name. The driver can trivially add the right offset.

> >>   - Don't register the SoC bus if the chip ID register is missing,
> >
> > Why? My objection was to hardcoding the register, not to registering
> > the device? I think I'd rather see the device registered with an
> > empty revision string.
> 
> If there's no chip ID register, there's no reason to use soc_device_match(),
> as we can always look at a compatible value. All SoCs listed in this driver
> have a chip ID register.

But you may still have user space tools looking into sysfs, e.g. to
figure out how to install a kernel that the boot loader can find,
or which hardware specific distro packages to install.

> if you want me to register the soc_bus for  those SoCs regardless, I want to
> re-add r7s72100 (RZ/A) and r8a7778 (R-Car M1A), who don't have chip ID
> registers ;-)

Right. Just don't encode too much knowledge about the SoCs into the
driver, so we are prepared for adding new ones: We should still look
for the registers in DT on all chips.

> >> +#define CCCR   0xe600101c  /* Common Chip Code Register */
> >> +#define PRR0xff44  /* Product Register */
> >> +#define PRR3   0xfff00044  /* Product Register on R-Car Gen3 */
> >> +
> >> +static const struct of_device_id renesas_socs[] __initconst = {
> >> +#ifdef CONFIG_ARCH_R8A73A4
> >> +   { .compatible = "renesas,r8a73a4",  .data = (void *)PRR, },
> >> +#endif
> >> +#ifdef CONFIG_ARCH_R8A7740
> >> +   { .compatible = "renesas,r8a7740",  .data = (void *)CCCR, },
> >> +#endif
> >
> > My preference here would be to list the register address only for
> > SoCs that are known to need them, while also having .dtb files that
> > don't have the nodes.
> 
> Even if drivers don't have to handle differences, there's been a long
> outstanding request to print SoC revision information during bootup
> (E.g. "Does it still work on ES1.0?"). Hence that covers all SoCs.

Ok, fair enough.

> >> +static int __init renesas_soc_init(void)
> >> +{
> >> +   struct soc_device_attribute *soc_dev_attr;
> >> +   const struct of_device_id *match;
> >> +   void __iom

Re: [PATCH] clk: fix link error for rcar-gen2

[Arnd Bergmann]

On Friday, October 21, 2016 8:01:49 PM CEST Geert Uytterhoeven wrote:
> > diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile
> > index 90dd0db7d9c6..762d122eddec 100644
> > --- a/drivers/clk/renesas/Makefile
> > +++ b/drivers/clk/renesas/Makefile
> > @@ -4,11 +4,7 @@ obj-$(CONFIG_ARCH_R8A73A4) += clk-r8a73a4.o 
> > clk-div6.o
> >  obj-$(CONFIG_ARCH_R8A7740) += clk-r8a7740.o clk-div6.o
> >  obj-$(CONFIG_ARCH_R8A7778) += clk-r8a7778.o
> >  obj-$(CONFIG_ARCH_R8A7779) += clk-r8a7779.o
> > -obj-$(CONFIG_ARCH_R8A7790) += clk-rcar-gen2.o clk-div6.o
> > -obj-$(CONFIG_ARCH_R8A7791) += clk-rcar-gen2.o clk-div6.o
> > -obj-$(CONFIG_ARCH_R8A7792) += clk-rcar-gen2.o clk-div6.o
> > -obj-$(CONFIG_ARCH_R8A7793) += clk-rcar-gen2.o clk-div6.o
> > -obj-$(CONFIG_ARCH_R8A7794) += clk-rcar-gen2.o clk-div6.o
> > +obj-$(CONFIG_ARCH_RCAR_GEN2)   += clk-rcar-gen2.o clk-div6.o
> >  obj-$(CONFIG_ARCH_R8A7795) += r8a7795-cpg-mssr.o 
> > rcar-gen3-cpg.o
> >  obj-$(CONFIG_ARCH_R8A7796) += r8a7796-cpg-mssr.o 
> > rcar-gen3-cpg.o
> >  obj-$(CONFIG_ARCH_SH73A0)  += clk-sh73a0.o clk-div6.o
> 
> Please don't fix it this way, as it will make it _harder_ to convert the R-Car
> Gen2 SoCs to the modern CPG/MSSR driver later.
> 

Ok, I see. How about just adding another line for r8a7743 and
dropping that again after the timer_init has been converted?

Arnd

Re: [PATCH/RFC 4/4] soc: renesas: Identify SoC and register with the SoC bus

[Arnd Bergmann]

On Friday, October 21, 2016 8:16:00 PM CEST Geert Uytterhoeven wrote:
> On Wed, Oct 19, 2016 at 12:59 PM, Arnd Bergmann <a...@arndb.de> wrote:
> > On Wednesday, October 19, 2016 10:02:57 AM CEST Geert Uytterhoeven wrote:
> >> On Mon, Oct 10, 2016 at 4:23 PM, Arnd Bergmann <a...@arndb.de> wrote:
> > I'd prefer seeing a separate soc driver for that one.
> >> Some SoCs have only CCCR, others have only PRR, some have both.
> >> On some SoCs one of them can be accessed from the RealTime CPU
> >> core (SH) only.
> >> On some SoCs the register is not documented, but present.
> >> If the PRR exists, it's a better choice, as it contains additional 
> >> information
> >> in the high order bits (representing the presence of each big (CA15/CA57),
> >> little (CA7/CA53), and RT (CR7) CPU core). Currently we don't use that
> >> information, though.
> >>
> >> Grouping them in some other way means we would loose the family name,
> >> which is exposed through soc_dev_attr->family.
> >> The usefulness of family names is debatable though, as this is more an
> >> issue of marketing business.
> >
> > How about having a table to look up the family name by the value
> > of the PRR or CCCR then?
> 
> Unfortunately there exist SoCs from different families using the same
> product ID.
> 
> And different SoCs from the same family may have a revision register
> or not (e.g. R-Car H1 has, M1A hasn't).

Is this something we expect to see more of in the future, or can
we expect future chips to handle this more consistently?

> > How about this:
> >
> > The driver could report the hardcoded strings for the SoCs it already
> > knows about (you have the table anyway) and not report the revision
> > unless there is a regmap containing the CCCR or the PRR, in which
> > case you use that. Future SoCs will provide the PRR (I assume
> > CCCR is only used on the older ones) through a syscon regmap
> > that we can use to find out the exact revision as well.
> >
> > The existing DT files can gain the syscon device so you can report
> > the revision on those machines as well, unless you use an old DTB.
> 
> Hmm... That means that if we have to add a driver quirk to distinguish
> between different revisions of the same SoC, we have to update the
> DTB anyway, to add the CCCR/PRR device node.
> We might as well just change the compatible value in that DTB for the
> device that needs the quirk. Which is what we'd like to avoid in the
> first place.

Do you have a specific example in mind? If this is only a theoretical
problem, we can worry about it when we get there, and then decide
if we add a hardcoded register after all.

> > Why not just drop all the #ifdef here? There should be very little
> > overhead in size, especially if all the data is __initconst.
> 
> It still saves ca. 3 KiB for a kernel for a single SoC.

Fair enough, that is more than I was expecting from looking at the
source. It's probably the of_device_id structures for the most part.

Please just add the __maybe_unused then, to save us a patch in case
we make -Wunused-const the default in the future.

Arnd

Re: [PATCH/RFC 4/4] soc: renesas: Identify SoC and register with the SoC bus

[Arnd Bergmann]

On Wednesday, October 19, 2016 10:02:57 AM CEST Geert Uytterhoeven wrote:
> On Mon, Oct 10, 2016 at 4:23 PM, Arnd Bergmann <a...@arndb.de> wrote:
> > On Tuesday, October 4, 2016 11:09:27 AM CEST Geert Uytterhoeven wrote:

> >> +static const struct renesas_family fam_rza __initconst = {
> >> + .name   = "RZ/A",
> >> +};
> >
> > I'm not sure about the relationship between this one and the others,
> > maybe it should be treated in the same way as emev2 and left out from
> > this driver?
> 
> While RZ/A doesn't have a version registers (AFAIK), it shares several
> drivers with the other SoCs (SH/R-Mobile, R-Car).
> Hence I'd like to keep it, so we can match for it in these drivers when
> needed. It has e.g. a different variant of the serial port (SCIF), more
> closely to the one on SH2 rather than SH4.

I'd prefer seeing a separate soc driver for that one.

> >> +static const struct renesas_family fam_rmobile __initconst = {
> >> + .name   = "R-Mobile",
> >> + .reg= 0xe600101c,   /* CCCR (Common Chip Code Register) 
> >> */
> >> +};
> >> +
> >> +static const struct renesas_family fam_rcar_gen1 __initconst = {
> >> + .name   = "R-Car Gen1",
> >> + .reg= 0xff44,   /* PRR (Product Register) */
> >> +};
> >> +
> >> +static const struct renesas_family fam_rcar_gen2 __initconst = {
> >> + .name   = "R-Car Gen2",
> >> + .reg= 0xff44,   /* PRR (Product Register) */
> >> +};
> >> +
> >> +static const struct renesas_family fam_rcar_gen3 __initconst = {
> >> + .name   = "R-Car Gen3",
> >> + .reg= 0xfff00044,   /* PRR (Product Register) */
> >> +};
> >> +
> >> +static const struct renesas_family fam_rzg __initconst = {
> >> + .name   = "RZ/G",
> >> + .reg= 0xff44,   /* PRR (Product Register) */
> >> +};
> >> +
> >> +static const struct renesas_family fam_shmobile __initconst = {
> >> + .name   = "SH-Mobile",
> >> + .reg= 0xe600101c,   /* CCCR (Common Chip Code Register) 
> >> */
> >> +};
> >
> > These seem to fall into two distinct categories, maybe there is a
> > better way to group them. What device contain the two kinds of
> > registers (PRR, CCCR)?
> 
> Actually there are three (notice the extra "f" on R-Car Gen3 ;-)

I see. Hopefully this is just the same register block at a different
location though.

> Some SoCs have only CCCR, others have only PRR, some have both.
> On some SoCs one of them can be accessed from the RealTime CPU
> core (SH) only.
> On some SoCs the register is not documented, but present.
> If the PRR exists, it's a better choice, as it contains additional information
> in the high order bits (representing the presence of each big (CA15/CA57),
> little (CA7/CA53), and RT (CR7) CPU core). Currently we don't use that
> information, though.
> 
> Grouping them in some other way means we would loose the family name,
> which is exposed through soc_dev_attr->family.
> The usefulness of family names is debatable though, as this is more an
> issue of marketing business.

How about having a table to look up the family name by the value
of the PRR or CCCR then?

> > Hardcoding the register address seems rather ugly here, so maybe
> > there is a way to have two separate probe methods based on the
> > surrounding register range, and then bind to that?
> 
> There's no simple relation between CCCR/PRR and other register blocks.
> I prefer not to add these to DT, as that would add one more worm to the
> backwards compatibility can.

Hmm, I understand the concern about compatibility with existing DT files,
but I also really hate to see hardcoded register addresses.

Any reason against requiring the DT node for future chips though?
How about this:

The driver could report the hardcoded strings for the SoCs it already
knows about (you have the table anyway) and not report the revision
unless there is a regmap containing the CCCR or the PRR, in which
case you use that. Future SoCs will provide the PRR (I assume
CCCR is only used on the older ones) through a syscon regmap
that we can use to find out the exact revision as well.

The existing DT files can gain the syscon device so you can report
the revision on those machines as well, unless you use an old DTB.

> >> +static const struct of_device_id renesas_socs[] __initconst = {
> >> +#ifdef CONFIG_ARCH_EMEV2
> >> + { .compatible = "renesas,emev2",

Re: [PATCH 0/4] soc: renesas: Identify SoC and register with the SoC bus

[Arnd Bergmann]

On Wednesday, October 19, 2016 10:10:38 AM CEST Geert Uytterhoeven wrote:
> Hi Arnd,
> 
> On Mon, Oct 10, 2016 at 4:28 PM, Arnd Bergmann <a...@arndb.de> wrote:
> > On Tuesday, October 4, 2016 11:09:23 AM CEST Geert Uytterhoeven wrote:
> >> Some Renesas SoCs may exist in different revisions, providing slightly
> >> different functionalities (e.g. R-Car H3 ES1.x and ES2.0). This needs to
> >> be catered for by drivers and/or platform code.  The recently proposed
> >> soc_device_match() API seems like a good fit to handle this.
> >>
> >> This patch series implements the core infrastructure to provide SoC and
> >> revision information through the SoC bus for Renesas ARM SoCs. It
> >> consists of 4 patches:
> >>   - Patch 1 avoids a crash when SoC revision information is needed and
> >> provided early,
> >>   - Patch 2 (from Arnd) introduces the soc_device_match() API.
> >> I don't know if, when, and through which channel this patch is
> >> planned to go upstream,
> >>   - Patch 3 fixes a bug in soc_device_match(), causing a crash when
> >> trying to match on an SoC attribute that is not provided (seen on
> >> EMEV2, RZ/A, and R-Car M1A, which lack revision information),
> >>   - Patch 4 identifies Renesas SoCs and registers them with the SoC bus.
> >>
> >> Tested on (family, machine, soc_id, optional revision):
> >>
> >> Emma Mobile EV2, EMEV2 KZM9D Board, emev2
> >> RZ/A, Genmai, r7s72100
> >> R-Mobile, APE6EVM, r8a73a4, ES1.0
> >> R-Mobile, armadillo 800 eva, r8a7740, ES2.0
> >> R-Car Gen1, bockw, r8a7778
> >> R-Car Gen1, marzen, r8a7779, ES1.0
> >> R-Car Gen2, Lager, r8a7790, ES1.0
> >> R-Car Gen2, Koelsch, r8a7791, ES1.0
> >> R-Car Gen2, Gose, r8a7793, ES1.0
> >> R-Car Gen2, Alt, r8a7794, ES1.0
> >> R-Car Gen3, Renesas Salvator-X board based on r8a7795, r8a7795, ES1.0
> >> R-Car Gen3, Renesas Salvator-X board based on r8a7796, r8a7796, ES1.0
> >> SH-Mobile, KZM-A9-GT, sh73a0, ES2.0
> >
> > As mentioned in the comment for the driver patch, I think this makes
> > a lot of sense for the machines that have a revision register, in
> > particular when the interpretation of that register is always done
> > the same way, but I'm a bit skeptical about doing it in the same driver
> > for machines that don't have the register.
> >
> > Matching by a device rather than the SoC platform also has the advantage
> > that there is no need to maintain a list of compatible numbers in the
> > driver.
> 
> Currently we (usually) use:
>   - SoC-specific compatible values, to handle known differences within the
> same family now, and handle future unknown differences,
>   - Family-specific compatible values, which we define ourselves.
> 
> Usually drivers match on the latter.
> 
> Every time a new SoC is introduced, we have to update lots of DT binding
> docs, to add the new SoC-specific compatible values.
>
> Two-phase matching (driver code matches against "renesas,",
> driver matches against SoC using soc_device_match()) would allow to
> remove the burden of updating DT documentation all the time.
> The drivers would need updates, though.
> Another advantage would be that we can reuse .dtsi snippets for SoCs in
> the same family, which we currently can't easily do due to the SoC-specific
> compatible values.

Interesting idea, but unrelated to my comment above, which was about
the soc driver in particular, rather the drivers using it.

Arnd

Re: [PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus

[Arnd Bergmann]

On Monday, November 14, 2016 11:51:15 AM CET Geert Uytterhoeven wrote:
> On Thu, Nov 10, 2016 at 12:37 PM, Arnd Bergmann <a...@arndb.de> wrote:
> > On Thursday, November 10, 2016 11:19:20 AM CET Geert Uytterhoeven wrote:
> >> On Wed, Nov 9, 2016 at 5:55 PM, Arnd Bergmann <a...@arndb.de> wrote:
> >> > On Monday, October 31, 2016 12:30:55 PM CET Geert Uytterhoeven wrote:
> >> >>   - Use "renesas,prr" and "renesas,cccr" device nodes in DT if
> >> >> available, else fall back to hardcoded addresses for compatibility
> >> >> with existing DTBs,
> 
> >> > It does seem wrong to have a device node for a specific register though.
> >> > Shouldn't the node be for the block of registers that these are inside
> >> > of?
> >>
> >> On R-Mobile APE6, R-Car Gen2 and Gen3, PRR is a lone register.
> >> On R-Car Gen1, it's not even documented (and doesn't exist on all parts).
> >
> > It just seems odd to have it at address 0xff44 when all the other
> > devices are at page-aligned addresses. Do you mean that accessing
> > 0xff40 or 0xff48 will result in a bus-level exception for a
> > missing register and just 0xff44 is actually valid for access,
> > or is it just the only thing that is documented?
> 
> For PRR, all other registers in the page read as all zeroes on all SoCs that
> have it. So it really is a lone register.

Ok.

> >> On SH-Mobile/R-Mobile, CCCR may be part of the HPB/APB register block, 
> >> which
> >> we further don't touch at all.
> >> On R-Car Gen2, it's not documented, but does exist.
> >
> > This is where the family names would come in handy ;-) I now have
> > no idea which chip(s) you are referring to.
> 
> SH/R-Mobile are r8a7740, r8a73a4, sh73a0.
> R-Car Gen2 are r8a779[0-4].
> 
> > If you know the name of the register block, just put it into DT with
> > that name. The driver can trivially add the right offset.
> 
> CCCR is different. The amount of registers that read as non-zero depends a lot
> on the actual SoC.
> 
> HPB/APB is gonna need real DT bindings, which needs some more investigation.
> Hence if you don't mind, I'd like to postpone that part, which only affects
> the older SoCs. And I'll drop the "renesas,cccr" binding.
> 
> For now, having revision detection for R-Car Gen3 (r8a779[56]) using PRR is
> most urgent, as several drivers (e.g. HDMI, Ethernet, clocks, pinctrl) are
> waiting for this support. So I'd like to have that dependency in v4.10.

Ok, sounds good.

> >> There is no SoC part number in the "renesas,prr" and "renesas,cccr" nodes.
> >> Hence I always need to look at the root nodes.
> >
> > Not sure what that would protect you from. Could you have a renesas,cccr
> 
> Looks like you forgot to finish your sentence?

Yes, and I forgot what I was going to say there now. It's probably covered
by what we discussed above.

Arnd

Re: [GIT PULL] Second Round of Renesas ARM Based SoC Drivers Updates for v4.10

[Arnd Bergmann]

On Friday, November 18, 2016 5:35:51 PM CET Olof Johansson wrote:
> 
> So, this pull request contains 8 patches, not 4. Seems like your pull
> request doesn't show any of the code from Geert's branch, didn't mention
> it in the tag and only in the email text above. Furthermore, Geert's
> branch modifies driver core code, so it's extra important to make sure
> it's clear that it's an unusual pull request.
> 
> Given that this modifies driver core, please either merge that code
> through Greg first, or get an ack from him. If you merge through him,
> make sure it's on a standalone topic branch that we can share.

As discussed on IRC, I think it makes sense to track that branch
separately in arm-soc, as its own top-level next/* branch.

The branch was first merged into the mmc tree after we got consensus
on the approach.

Arnd

[PATCH] clk: renesas: cpg_mstp: build for R8A774{3,5}

[Arnd Bergmann]

We get a link failure on R8A7745 and R8A7743 when the other platforms
are disabled and nothing selects the clk-rcar-gen2 driver:

drivers/clk/renesas/clk-rcar-gen2.o: In function `rcar_gen2_cpg_clocks_init':
clk-rcar-gen2.c:(.init.text+0x39c): undefined reference to 
`cpg_mstp_add_clk_domain'

This adds another 'select' statement to get them to build as well.

Fixes: 9127d54bb894 ("clk: renesas: cpg-mssr: Add R8A7745 support")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/clk/renesas/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig
index 2586dfa0026b..4ebd9d3d24b7 100644
--- a/drivers/clk/renesas/Kconfig
+++ b/drivers/clk/renesas/Kconfig
@@ -10,6 +10,8 @@ config CLK_RENESAS_CPG_MSTP
default y if ARCH_R7S72100
default y if ARCH_R8A73A4
default y if ARCH_R8A7740
+   default y if ARCH_R8A7743
+   default y if ARCH_R8A7745
default y if ARCH_R8A7778
default y if ARCH_R8A7779
default y if ARCH_R8A7790
-- 
2.9.0

Re: [PATCH] clk: renesas: cpg_mstp: build for R8A774{3,5}

[Arnd Bergmann]

On Thursday, November 24, 2016 5:38:45 PM CET Geert Uytterhoeven wrote:
> 
> R8A7745 and R8A7743 don't need the CPG_MSTP driver, they select
> CLK_RENESAS_CPG_MSSR instead.
> 
> Perhaps you still have your patch "[PATCH] clk: fix link error for rcar-gen2"
> applied to enable clk-rcar-gen2.o on R8A7745?
> 
> Gr{oetje,eeting}s,
> 

Ah, right. My mistake. I'll drop both then.

Arnd

[PATCH 1/3] [media] v4l: rcar_fdp1: mark PM functions as __maybe_unused

[Arnd Bergmann]

The new driver produces a warning when CONFIG_PM is disabled:

platform/rcar_fdp1.c:2408:12: error: 'fdp1_pm_runtime_resume' defined but not 
used [-Werror=unused-function]
platform/rcar_fdp1.c:2399:12: error: 'fdp1_pm_runtime_suspend' defined but not 
used [-Werror=unused-function]

This marks the two functions as __maybe_unused.

Fixes: 4710b752e029 ("[media] v4l: Add Renesas R-Car FDP1 Driver")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/media/platform/rcar_fdp1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rcar_fdp1.c 
b/drivers/media/platform/rcar_fdp1.c
index dd1a6ea17f22..674cc1309b43 100644
--- a/drivers/media/platform/rcar_fdp1.c
+++ b/drivers/media/platform/rcar_fdp1.c
@@ -2396,7 +2396,7 @@ static int fdp1_remove(struct platform_device *pdev)
return 0;
 }
 
-static int fdp1_pm_runtime_suspend(struct device *dev)
+static int __maybe_unused fdp1_pm_runtime_suspend(struct device *dev)
 {
struct fdp1_dev *fdp1 = dev_get_drvdata(dev);
 
@@ -2405,7 +2405,7 @@ static int fdp1_pm_runtime_suspend(struct device *dev)
return 0;
 }
 
-static int fdp1_pm_runtime_resume(struct device *dev)
+static int __maybe_unused fdp1_pm_runtime_resume(struct device *dev)
 {
struct fdp1_dev *fdp1 = dev_get_drvdata(dev);
 
-- 
2.9.0

Re: [RFC PATCH] of: base: add support to get machine model name

[Arnd Bergmann]

On Thursday, November 17, 2016 2:08:30 PM CET Sudeep Holla wrote:
> On 17/11/16 13:50, Arnd Bergmann wrote:
> > On Thursday, November 17, 2016 11:50:50 AM CET Sudeep Holla wrote:
> >> Currently platforms/drivers needing to get the machine model name are
> >> replicating the same snippet of code. In some case, the OF reference
> >> counting is either missing or incorrect.
> >>
> >> This patch adds support to read the machine model name either using
> >> the "model" or the "compatible" property in the device tree root node.
> >>
> >> Signed-off-by: Sudeep Holla <sudeep.ho...@arm.com>
> >
> > I like the idea. One small comment:
> >
> 
> Thanks. I prefer it as single patch but it can't be applied to any tree.
> Any suggestions on handling this patch to fix the warning in -next ?
> 
The patch that causes the warning is currently in the mmc tree, and I
don't think it would be good to have your entire patch in there too.

It's probably best to just fix the warning there now by adding another
open-coded copy of that function, and then apply your patch on top
for v4.11.

Arnd

Re: [RFC PATCH] of: base: add support to get machine model name

[Arnd Bergmann]

On Thursday, November 17, 2016 11:50:50 AM CET Sudeep Holla wrote:
> Currently platforms/drivers needing to get the machine model name are
> replicating the same snippet of code. In some case, the OF reference
> counting is either missing or incorrect.
> 
> This patch adds support to read the machine model name either using
> the "model" or the "compatible" property in the device tree root node.
> 
> Signed-off-by: Sudeep Holla 

I like the idea. One small comment:

> +int of_machine_get_model_name(const char **model)
> +{
> + int error;
> + struct device_node *root;
> +
> + root = of_find_node_by_path("/");
> + if (!root)
> + return -EINVAL;

The global of_root variable points ot this already, and is defined
in the same file, so I think we can just skip the lookup.

Arnd

Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

[Arnd Bergmann]

On Tuesday, January 10, 2017 3:44:53 PM CET Christoph Hellwig wrote:
> On Tue, Jan 10, 2017 at 11:47:42AM +0100, Arnd Bergmann wrote:
> > I see that we have CONFIG_ARCH_PHYS_ADDR_T_64BIT on a couple of
> > 32-bit architectures without swiotlb (arc, arm, some mips32), and
> > there are several 64-bit architectures that do not have swiotlb
> > (alpha, parisc, s390, sparc). I believe that alpha, s390 and sparc
> > always use some form of IOMMU, but the other four apparently don't,
> > so we would need to add swiotlb support there to remove all the
> > bounce buffering in network and block layers.
> 
> mips has lots of weird swiotlb wire-up in it's board code (the swiotlb
> arch glue really needs some major cleanup..),

My reading of the MIPS code was that only the 64-bit platforms use it,
but there are a number of 32-bit platforms that have 64-bit physical
addresses and don't.

> as does arm.  Not sure about the others.

32-bit ARM doesn't actually use SWIOTLB at all, despite selecting it
in Kconfig. I think Xen uses it for its own purposes, but nothing
else does.

Most ARM platforms can't actually have RAM beyond 4GB, and the ones
that do have it tend to also come with an IOMMU, but I remember
at least BCM53xx actually needing swiotlb on some chip revisions
that are widely used and that cannot DMA to the second memory bank
from PCI (!).

Arnd

Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

[Arnd Bergmann]

On Wednesday, January 11, 2017 3:37:22 PM CET Nikita Yushchenko wrote:
> > I actually have a third variation of this problem involving a PCI root
> > complex which *could* drive full-width (40-bit) addresses, but won't,
> > due to the way its PCI<->AXI interface is programmed. That would require
> > even more complicated dma-ranges handling to describe the windows of
> > valid physical addresses which it *will* pass, so I'm not pressing the
> > issue - let's just get the basic DMA mask case fixed first.
> 
> R-Car + NVMe is actually not "basic case".
> 
> It has PCI<->AXI interface involved.
> PCI addresses are 64-bit and controller does handle 64-bit addresses
> there. Mapping between PCI addresses and AXI addresses is defined. But
> AXI is 32-bit.
> 
> SoC has iommu that probably could be used between PCIe module and RAM.
> Although AFAIK nobody made that working yet.
> 
> Board I work with has 4G of RAM, in 4 banks, located at different parts
> of wide address space, and only one of them is below 4G. But if iommu is
> capable of translating addresses such that 4 gigabyte banks map to first
> 4 gigabytes of address space, then all memory will become available for
> DMA from PCIe device.

You can in theory handle this by defining your own platform specific
dma_map_ops, as we used to do in the old days. Unfortunately, the modern
way of using the generic IOVA allocation can't handle really it, so it's
unclear if the work that would be necessary to support it (and the long
term maintenance cost) outweigh the benefits.

The more likely option here is to try harder to get the IOMMU working
(or show that it's impossible but make sure the next chip gets it right).

Arnd

Re: NVMe vs DMA addressing limitations

[Arnd Bergmann]

On Thursday, January 12, 2017 12:09:11 PM CET Sagi Grimberg wrote:
> >> Another workaround me might need is to limit amount of concurrent DMA
> >> in the NVMe driver based on some platform quirk. The way that NVMe works,
> >> it can have very large amounts of data that is concurrently mapped into
> >> the device.
> >
> > That's not really just NVMe - other storage and network controllers also
> > can DMA map giant amounts of memory.  There are a couple aspects to it:
> >
> >  - dma coherent memoery - right now NVMe doesn't use too much of it,
> >but upcoming low-end NVMe controllers will soon start to require
> >fairl large amounts of it for the host memory buffer feature that
> >allows for DRAM-less controller designs.  As an interesting quirk
> >that is memory only used by the PCIe devices, and never accessed
> >by the Linux host at all.
> 
> Would it make sense to convert the nvme driver to use normal allocations
> and use the DMA streaming APIs (dma_sync_single_for_[cpu|device]) for
> both queues and future HMB?

That is an interesting question: We actually have the
"DMA_ATTR_NO_KERNEL_MAPPING" for this case, and ARM implements
it in the coherent interface, so that might be a good fit.

Implementing it in the streaming API makes no sense since we
already have a kernel mapping here, but using a normal allocation
(possibly with DMA_ATTR_NON_CONSISTENT or DMA_ATTR_SKIP_CPU_SYNC,
need to check) might help on other architectures that have
limited amounts of coherent memory and no CMA.

Another benefit of the coherent API for this kind of buffer is
that we can use CMA where available to get a large consecutive
chunk of RAM on architectures without an IOMMU when normal
memory is no longer available because of fragmentation.

Arnd

Re: [PATCH 1/2] dma-mapping: let arch know origin of dma range passed to arch_setup_dma_ops()

[Arnd Bergmann]

On Thursday, January 12, 2017 9:33:32 AM CET Nikita Yushchenko wrote:
> >> Hmm, I think when the dma-ranges are missing, we should either enforce
> >> a 32-bit mask, or disallow DMA completely. It's probably too late for
> >> the latter, I wish we had done this earlier in order to force everyone
> >> on ARM64 to have a valid dma-ranges property for any DMA master.
> > 
> > This can be done over time.
> > 
> > However the very idea of this version of patch is - keep working pieces
> > as-is, thus for now setting enforce_range to false in case of no defined
> > dma-ranges is intentional.
> 
> What we can do is - check bus width (as it is defined in DT) and set
> enforce_range to true if bus is 32-bit
> 
> > What I should re-check is - does rcar dtsi set dma-ranges, and add it if
> > it does not.
> 
> It does not, will have to add.
> 
> In DT bus is defined as 64-bit. But looks like physically it is 32-bit.
> Maybe DT needs fixing.

I think we always assumed that the lack of a dma-ranges property
implied a 32-bit width, as that is the safe fallback as well as the
most common case.

AFAICT, this means you are actually fine on rcar, and all other
platforms will keep working as we enforce it, but might get slowed
down if they relied on the unintended behavior of allowing 64-bit
DMA.

Arnd

Re: [PATCH 1/2] dma-mapping: let arch know origin of dma range passed to arch_setup_dma_ops()

[Arnd Bergmann]

On Thursday, January 12, 2017 12:16:24 PM CET Will Deacon wrote:
> On Thu, Jan 12, 2017 at 08:52:51AM +0300, Nikita Yushchenko wrote:
> > >> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c 
> > >> b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> > >> index 5ac373c..480b644 100644
> > >> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> > >> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> > >> @@ -540,7 +540,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
> > >>  
> > >>/* Objects are coherent, unless 'no shareability' flag set. */
> > >>if (!(obj_desc->flags & DPRC_OBJ_FLAG_NO_MEM_SHAREABILITY))
> > >> -  arch_setup_dma_ops(_dev->dev, 0, 0, NULL, true);
> > >> +  arch_setup_dma_ops(_dev->dev, 0, 0, false, NULL, true);
> > >>  
> > >>/*
> > >> * The device-specific probe callback will get invoked by device_add()
> > > 
> > > Why are these actually calling arch_setup_dma_ops() here in the first
> > > place? Are these all devices that are DMA masters without an OF node?
> > 
> > I don't know, but that's a different topic. This patch just adds
> > argument and sets it to false everywhere but in the location when range
> > should be definitely enforced.
> 
> I also wouldn't lose any sleep over a staging driver.

I think this is in the process of being moved out of staging, and
my question was about the other two as well:

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
drivers/iommu/rockchip-iommu.c

Arnd

Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

[Arnd Bergmann]

On Tuesday, January 10, 2017 1:25:12 PM CET Robin Murphy wrote:
> On 10/01/17 12:47, Nikita Yushchenko wrote:
> >> The point here is that an IOMMU doesn't solve your issue, and the
> >> IOMMU-backed DMA ops need the same treatment. In light of that, it really
> >> feels to me like the DMA masks should be restricted in of_dma_configure
> >> so that the parent mask is taken into account there, rather than hook
> >> into each set of DMA ops to intercept set_dma_mask. We'd still need to
> >> do something to stop dma_set_mask widening the mask if it was restricted
> >> by of_dma_configure, but I think Robin (cc'd) was playing with that.
> > 
> > What issue "IOMMU doesn't solve"?
> > 
> > Issue I'm trying to address is - inconsistency within swiotlb
> > dma_map_ops, where (1) any wide mask is silently accepted, but (2) then
> > mask is used to decide if bounce buffers are needed or not. This
> > inconsistency causes NVMe+R-Car cobmo not working (and breaking memory
> > instead).
> 
> The fundamental underlying problem is the "any wide mask is silently
> accepted" part, and that applies equally to IOMMU ops as well.

It's a much rarer problem for the IOMMU case though, because it only
impacts devices that are restricted to addressing of less than 32-bits.

If you have an IOMMU enabled, the dma-mapping interface does not care
if the device can do wider than 32 bit addressing, as it will never
hand out IOVAs above 0x.

> > I just can't think out what similar issue iommu can have.
> > Do you mean that in iommu case, mask also must not be set to whatever
> > wider than initial value? Why? What is the use of mask in iommu case? Is
> > there any real case when iommu can't address all memory existing in the
> > system?
> 
> There's a very subtle misunderstanding there - the DMA mask does not
> describe the memory a device can address, it describes the range of
> addresses the device is capable of generating. Yes, in the non-IOMMU
> case they are equivalent, but once you put an IOMMU in between, the
> problem is merely shifted from "what range of physical addresses can
> this device access" to "what range of IOVAs is valid to give to this
> device" - the fact that those IOVAs can map to any underlying physical
> address only obviates the need for any bouncing at the memory end; it
> doesn't remove the fact that the device has a hardware addressing
> limitation which needs to be accommodated.
> 
> The thread Will linked to describes that equivalent version of your
> problem - the IOMMU gives the device 48-bit addresses which get
> erroneously truncated because it doesn't know that only 42 bits are
> actually wired up. That situation still requires the device's DMA mask
> to correctly describe its addressing capability just as yours does.

That problem should only impact virtual machines which have a guest
bus address space covering more than 42 bits of physical RAM, whereas
the problem we have with swiotlb is for the dma-mapping interface.

> > With this direction, semantics of dma mask becomes even more
> > questionable. I'd say dma_mask is candidate for removal (or to move to
> > swiotlb's or iommu's local area)
> 
> We still need a way for drivers to communicate a device's probed
> addressing capability to SWIOTLB, so there's always going to have to be
> *some* sort of public interface. Personally, the change in semantics I'd
> like to see is to make dma_set_mask() only fail if DMA is entirely
> disallowed - in the normal case it would always succeed, but the DMA API
> implementation would be permitted to set a smaller mask than requested
> (this is effectively what the x86 IOMMU ops do already).

With swiotlb enabled, it only needs to fail if the mask does not contain
the swiotlb bounce buffer area, either because the start of RAM is outside
of the mask, or the bounce area has been allocated at the end of ZONE_DMA
and the mask is smaller than ZONE_DMA.

Arnd

Re: NVMe vs DMA addressing limitations

[Arnd Bergmann]

On Tuesday, January 10, 2017 3:48:39 PM CET Christoph Hellwig wrote:
> On Tue, Jan 10, 2017 at 12:01:05PM +0100, Arnd Bergmann wrote:
> > Another workaround me might need is to limit amount of concurrent DMA
> > in the NVMe driver based on some platform quirk. The way that NVMe works,
> > it can have very large amounts of data that is concurrently mapped into
> > the device.
> 
> That's not really just NVMe - other storage and network controllers also
> can DMA map giant amounts of memory.  There are a couple aspects to it:
> 
>  - dma coherent memoery - right now NVMe doesn't use too much of it,
>but upcoming low-end NVMe controllers will soon start to require
>fairl large amounts of it for the host memory buffer feature that
>allows for DRAM-less controller designs.  As an interesting quirk
>that is memory only used by the PCIe devices, and never accessed
>by the Linux host at all.

Right, that is going to become interesting, as some platforms are
very limited with their coherent allocations.

>  - size vs number of the dynamic mapping.  We probably want the dma_ops
>specify a maximum mapping size for a given device.  As long as we
>can make progress with a few mappings swiotlb / the iommu can just
>fail mapping and the driver will propagate that to the block layer
>that throttles I/O.

Good idea.

Arnd

Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

[Arnd Bergmann]

On Tuesday, January 10, 2017 2:16:57 PM CET Robin Murphy wrote:
> On 10/01/17 13:42, Arnd Bergmann wrote:
> > On Tuesday, January 10, 2017 1:25:12 PM CET Robin Murphy wrote:
> >> On 10/01/17 12:47, Nikita Yushchenko wrote:
> >>>> The point here is that an IOMMU doesn't solve your issue, and the
> >>>> IOMMU-backed DMA ops need the same treatment. In light of that, it really
> >>>> feels to me like the DMA masks should be restricted in of_dma_configure
> >>>> so that the parent mask is taken into account there, rather than hook
> >>>> into each set of DMA ops to intercept set_dma_mask. We'd still need to
> >>>> do something to stop dma_set_mask widening the mask if it was restricted
> >>>> by of_dma_configure, but I think Robin (cc'd) was playing with that.
> >>>
> >>> What issue "IOMMU doesn't solve"?
> >>>
> >>> Issue I'm trying to address is - inconsistency within swiotlb
> >>> dma_map_ops, where (1) any wide mask is silently accepted, but (2) then
> >>> mask is used to decide if bounce buffers are needed or not. This
> >>> inconsistency causes NVMe+R-Car cobmo not working (and breaking memory
> >>> instead).
> >>
> >> The fundamental underlying problem is the "any wide mask is silently
> >> accepted" part, and that applies equally to IOMMU ops as well.
> > 
> > It's a much rarer problem for the IOMMU case though, because it only
> > impacts devices that are restricted to addressing of less than 32-bits.
> > 
> > If you have an IOMMU enabled, the dma-mapping interface does not care
> > if the device can do wider than 32 bit addressing, as it will never
> > hand out IOVAs above 0x.
> 
> I can assure you that it will - we constrain allocations to the
> intersection of the IOMMU domain aperture (normally the IOMMU's physical
> input address width) and the given device's DMA mask. If both of those
> are >32 bits then >32-bit IOVAs will fall out. For the arm64/common
> implementation I have prototyped a copy of the x86 optimisation which
> always first tries to get 32-bit IOVAs for PCI devices, but even then it
> can start returning higher addresses if the 32-bit space fills up.

Ok, got it. I have to admit that most of my knowledge about the internals
of IOMMUs is from PowerPC of a few years ago, which couldn't do this at
all. I agree that we need to do the same thing on swiotlb and iommu then.

> >> The thread Will linked to describes that equivalent version of your
> >> problem - the IOMMU gives the device 48-bit addresses which get
> >> erroneously truncated because it doesn't know that only 42 bits are
> >> actually wired up. That situation still requires the device's DMA mask
> >> to correctly describe its addressing capability just as yours does.
> > 
> > That problem should only impact virtual machines which have a guest
> > bus address space covering more than 42 bits of physical RAM, whereas
> > the problem we have with swiotlb is for the dma-mapping interface.
> > 
> I actually have a third variation of this problem involving a PCI root
> complex which *could* drive full-width (40-bit) addresses, but won't,
> due to the way its PCI<->AXI interface is programmed. That would require
> even more complicated dma-ranges handling to describe the windows of
> valid physical addresses which it *will* pass, so I'm not pressing the
> issue - let's just get the basic DMA mask case fixed first.

Can you describe this a little more? We should at least try to not
make it harder to solve the next problem while solving this one,
so I'd like to understand the exact limitation you are hitting there.

Arnd

Re: [PATCH 1/2] dma-mapping: let arch know origin of dma range passed to arch_setup_dma_ops()

[Arnd Bergmann]

On Wednesday, January 11, 2017 9:31:51 PM CET Nikita Yushchenko wrote:

> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 9afcbf7..0995ab3 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -1096,7 +1096,7 @@ static int rk_iommu_domain_probe(struct platform_device 
> *pdev)
>   return -ENOMEM;
>  
>   /* Set dma_ops for dev, otherwise it would be dummy_dma_ops */
> - arch_setup_dma_ops(dev, 0, DMA_BIT_MASK(32), NULL, false);
> + arch_setup_dma_ops(dev, 0, DMA_BIT_MASK(32), false, NULL, false);
>  
>   dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
>   dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c 
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index c9b7ad6..19f70d8 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -2533,7 +2533,7 @@ static int dpaa_eth_probe(struct platform_device *pdev)
>   priv->buf_layout[TX].priv_data_size = DPAA_TX_PRIV_DATA_SIZE; /* Tx */
>  
>   /* device used for DMA mapping */
> - arch_setup_dma_ops(dev, 0, 0, NULL, false);
> + arch_setup_dma_ops(dev, 0, 0, false, NULL, false);
>   err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(40));
>   if (err) {
>   dev_err(dev, "dma_coerce_mask_and_coherent() failed\n");
> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c 
> b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> index 5ac373c..480b644 100644
> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> @@ -540,7 +540,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>  
>   /* Objects are coherent, unless 'no shareability' flag set. */
>   if (!(obj_desc->flags & DPRC_OBJ_FLAG_NO_MEM_SHAREABILITY))
> - arch_setup_dma_ops(_dev->dev, 0, 0, NULL, true);
> + arch_setup_dma_ops(_dev->dev, 0, 0, false, NULL, true);
>  
>   /*
>* The device-specific probe callback will get invoked by device_add()

Why are these actually calling arch_setup_dma_ops() here in the first
place? Are these all devices that are DMA masters without an OF node?

> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index fd5cfad..1cc2115 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -89,6 +89,7 @@ void of_dma_configure(struct device *dev, struct 
> device_node *np)
>   bool coherent;
>   unsigned long offset;
>   const struct iommu_ops *iommu;
> + bool enforce_range = false;
>  
>   /*
>* Set default coherent_dma_mask to 32 bit.  Drivers are expected to
> @@ -126,6 +127,8 @@ void of_dma_configure(struct device *dev, struct 
> device_node *np)
>   return;
>   }
>   dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
> +
> + enforce_range = true;
>   }
>  
>   dev->dma_pfn_offset = offset;

Hmm, I think when the dma-ranges are missing, we should either enforce
a 32-bit mask, or disallow DMA completely. It's probably too late for
the latter, I wish we had done this earlier in order to force everyone
on ARM64 to have a valid dma-ranges property for any DMA master.

Arnd

Re: [GIT PULL] Renesas ARM Based SoC r8a7745 SYSC Driver Updates for v4.10

[Arnd Bergmann]

On Wednesday, November 23, 2016 9:13:10 PM CET Simon Horman wrote:
> Hi Olof, Hi Kevin, Hi Arnd,
> 
> Please consider these Renesas ARM based SoC r8a7745 SYSC Driver updates for
> v4.10.
> 
> This pull request is based on v4.9-rc1.
> 
> This pull request is broken out of an earlier pull request,
> "Second Round of Renesas ARM Based SoC Drivers Updates for v4.10".
> The motivation for breaking up the pull request is to provide
> branches with minimal dependencies.
> 
> This pull request is targeted at the next/drivers branch of the arm-soc tree.
> 
> It introduces some minor merge conflicts in:
> Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt
> drivers/soc/renesas/Makefile
> drivers/soc/renesas/rcar-sysc.c
> drivers/soc/renesas/rcar-sysc.h
> The resolution is to take both sides. A sample merge resolution is
> provided in the renesas-next-20161123v2-v4.9-rc1 tag of the renesas tree.
> 

Pulled into next/drivers, thanks!

Arnd


diff --cc Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt
index c16ec18,1375758..d91715b
--- a/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt
+++ b/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt
@@@ -7,7 -7,7 +7,8 @@@ and various coprocessors
  
  Required properties:
- compatible: Must contain exactly one of the following:
 +  - "renesas,r8a7743-sysc" (RZ/G1M)
+   - "renesas,r8a7745-sysc" (RZ/G1E)
- "renesas,r8a7779-sysc" (R-Car H1)
- "renesas,r8a7790-sysc" (R-Car H2)
- "renesas,r8a7791-sysc" (R-Car M2-W)
diff --cc drivers/soc/renesas/Makefile
index 9e0bb32,a25a628..e2249f0
--- a/drivers/soc/renesas/Makefile
+++ b/drivers/soc/renesas/Makefile
@@@ -1,4 -1,4 +1,5 @@@
 +obj-$(CONFIG_ARCH_R8A7743)+= rcar-sysc.o r8a7743-sysc.o
+ obj-$(CONFIG_ARCH_R8A7745)+= rcar-sysc.o r8a7745-sysc.o
  obj-$(CONFIG_ARCH_R8A7779)+= rcar-sysc.o r8a7779-sysc.o
  obj-$(CONFIG_ARCH_R8A7790)+= rcar-sysc.o r8a7790-sysc.o
  obj-$(CONFIG_ARCH_R8A7791)+= rcar-sysc.o r8a7791-sysc.o
diff --cc drivers/soc/renesas/rcar-sysc.c
index 71acd45,a4e1bb0..225c35c
--- a/drivers/soc/renesas/rcar-sysc.c
+++ b/drivers/soc/renesas/rcar-sysc.c
@@@ -275,9 -275,9 +275,12 @@@ finalize
  }
  
  static const struct of_device_id rcar_sysc_matches[] = {
 +#ifdef CONFIG_ARCH_R8A7743
 +  { .compatible = "renesas,r8a7743-sysc", .data = _sysc_info },
 +#endif
+ #ifdef CONFIG_ARCH_R8A7745
+   { .compatible = "renesas,r8a7745-sysc", .data = _sysc_info },
+ #endif
  #ifdef CONFIG_ARCH_R8A7779
{ .compatible = "renesas,r8a7779-sysc", .data = _sysc_info },
  #endif
diff --cc drivers/soc/renesas/rcar-sysc.h
index 8ab9ca8,3048dd3..f6e842e
--- a/drivers/soc/renesas/rcar-sysc.h
+++ b/drivers/soc/renesas/rcar-sysc.h
@@@ -50,7 -50,7 +50,8 @@@ struct rcar_sysc_info 
unsigned int num_areas;
  };
  
 +extern const struct rcar_sysc_info r8a7743_sysc_info;
+ extern const struct rcar_sysc_info r8a7745_sysc_info;
  extern const struct rcar_sysc_info r8a7779_sysc_info;
  extern const struct rcar_sysc_info r8a7790_sysc_info;
  extern const struct rcar_sysc_info r8a7791_sysc_info;

Re: [git pull] soc_device_match() interface for matching against soc_bus attributes

[Arnd Bergmann]

On Wednesday, November 23, 2016 1:32:56 PM CET Geert Uytterhoeven wrote:
> soc_device_match() interface for matching against soc_bus attributes
> 
> This provides core infrastructure as a dependency for several users
> (Freescale/NXP, Samsung, Renesas).
> 
> Its core parts have been acked by Greg, and the fixes by Arnd and/or
> Greg (the last fix only received an informal ack, that's why I hadn't
> added the ack).
> 
> This has already been pulled by Ulf, and is present in mmc/next, as a
> dependency for a Freescale/NXP driver update.
> 
> 

I've pulled this into next/drivers and also into a separate 
next/socdevice-match that we can use for keeping track. Thanks,

Arnd

Re: [git pull] Renesas RZ/G1M and RZ/G1E CPG Core Clock Definitions

[Arnd Bergmann]

On Wednesday, November 23, 2016 1:39:58 PM CET Geert Uytterhoeven wrote:
> Renesas RZ/G1M and RZ/G1E CPG Core Clock Definitions
> 
> This contains clock definitions shared by clock drivers and DTS files,
> and is provided as a dependency for the latter.
> 
> This has already been pulled by Stephen as part of a larger pull
> request, and is present in clk/clk-next, together with the corresponding
> clock drivers.
> 

Pulled into next/dt, thanks!

Arnd

Re: [GIT PULL v2] Second Round of Renesas ARM Based SoC DT Updates for v4.10

[Arnd Bergmann]

On Wednesday, November 23, 2016 9:14:33 PM CET Simon Horman wrote:
> Second Round of Renesas ARM Based SoC DT Updates for v4.10
> 
> Enhancements:
> * Add device nodes for PRR
> * Add r8a7745 SoC and sk-rzg1e board
> * Add r8a7743 SoC and sk-rzg1m board
> * Enable SDR-104 and I2C demuxer on alt, koelsch and lager boards
> 
> Corrections:
> * Use SYSC "always-on" PM Domain for sound on r8a7794 SoC
> * Correct hsusb parent clock on r8a7794 SoC
> * Correct PFC names for DU on alt board
> 

Pulled into next/dt, thanks!

Arnd

Re: [GIT PULL v2] Second Round of Renesas ARM64 Based SoC DT Updates for v4.10

[Arnd Bergmann]

On Monday, November 21, 2016 1:05:10 PM CET Simon Horman wrote:
> Second Round of Renesas ARM64 Based SoC DT Updates for v4.10
> 
> Enhancements:
> * Add device nodes for PRR
> * Add m3ulcb board
> * Enable I2C on r8a7796/salvator-x board
> * Enable SDHI0 on h3ulcb board
> 
> 

Pulled into next/dt64, thanks!

Arnd

Re: [GIT PULL] Renesas ARM Based SoC Match Updates for v4.10

[Arnd Bergmann]

On Wednesday, November 23, 2016 9:13:31 PM CET Simon Horman wrote:
> This pull request is based on the pull request "soc_device_match()
> interface for matching against soc_bus attributes" sent by Geert
> Uytterhoeven which corresponds to the soc-device-match-tag1 tag in his
> renesas-driver's tree. It provides infrastructure used by changes in this
> pull request.
> 
> This pull request is broken out of an earlier pull request,
> "Second Round of Renesas ARM Based SoC Drivers Updates for v4.10".
> The motivation for breaking up the pull request is to provide
> branches with minimal dependencies.
> 
> This pull request is targeted at the next/drivers branch of the arm-soc tree.
> 
> It introduces some minor merge conflicts in drivers/soc/renesas/Makefile.
> The resolution is to take both sides.  A sample merge resolution is
> provided in the renesas-next-20161123v2-v4.9-rc1 tag of the renesas tree.
> 

Pulled into next/drivers, thanks!

Arnd

Re: [GIT PULL] Renesas ARM Based SoC Match Updates for v4.10

[Arnd Bergmann]

On Thursday, December 1, 2016 9:47:58 AM CET Simon Horman wrote:

> Thanks for pulling this and other outstanding pull requests from Geert and
> myself. I don't think we have anything more outstanding at this time.

Ok, thanks for the confirmation!

Arnd

Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

[Arnd Bergmann]

On Wednesday, January 4, 2017 6:29:39 PM CET Nikita Yushchenko wrote:

> > Just a guess, but if the inbound translation windows in the host
> > bridge are wider than 32-bit, the reason for setting up a single
> > 32-bit window is probably because that is what the parent bus supports.
> 
> Well anyway applying patch similar to your's will fix pcie-rcar + nvme
> case - thus I don't object :)   But it can break other cases ...
> 
> But why do you hook at set_dma_mask() and overwrite mask inside, instead
> of hooking at dma_supported() and rejecting unsupported mask?
> 
> I think later is better, because it lets drivers to handle unsupported
> high-dma case, like documented in DMA-API_HOWTO.

I think the behavior I put in there is required for swiotlb to make
sense, otherwise you would rely on the driver to handle dma_set_mask()
failure gracefully with its own bounce buffers (as network and
scsi drivers do but others don't).

Having swiotlb or iommu enabled should result in dma_set_mask() always
succeeding unless the mask is too small to cover the swiotlb
bounce buffer area or the iommu virtual address space. This behavior
is particularly important in case the bus address space is narrower
than 32-bit, as we have to guarantee that the fallback to 32-bit
DMA always succeeds. There are also a lot of drivers that try to
set a 64-bit mask but don't implement bounce buffers for streaming
mappings if that fails, and swiotlb is what we use to make those
drivers work.

And yes, the API is a horrible mess.

Arnd

Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

[Arnd Bergmann]

On Wednesday, January 4, 2017 9:24:09 AM CET Nikita Yushchenko wrote:
> > commit 9a57d58d116800a535510053136c6dd7a9c26e25
> > Author: Arnd Bergmann <a...@arndb.de>
> > Date:   Tue Nov 17 14:06:55 2015 +0100
> > 
> > [EXPERIMENTAL] ARM64: check implement dma_set_mask
> > 
> > Needs work for coherent mask
> > 
> > Signed-off-by: Arnd Bergmann <a...@arndb.de>
> 
> Unfortunately this is far incomplete
> 
> > @@ -957,6 +983,18 @@ void arch_setup_dma_ops(struct device *dev, u64 
> > dma_base, u64 size,
> > if (!dev->archdata.dma_ops)
> > dev->archdata.dma_ops = _dma_ops;
> >  
> > +   /*
> > +* we don't yet support buses that have a non-zero mapping.
> > +*  Let's hope we won't need it
> > +*/
> > +   WARN_ON(dma_base != 0);
> > +
> > +   /*
> > +* Whatever the parent bus can set. A device must not set
> > +* a DMA mask larger than this.
> > +*/
> > +   dev->archdata.parent_dma_mask = size;
> > +
> 
> ... because size/mask passed here for PCI devices are meaningless.
> 
> For OF platforms, this is called via of_dma_configure(), that checks
> dma-ranges of node that is *parent* for host bridge. Host bridge
> currently does not control this at all.

We need to think about this a bit. Is it actually the PCI host
bridge that limits the ranges here, or the bus that it is connected
to. In the latter case, the caller needs to be adapted to handle
both.

> In current device trees no dma-ranges is defined for nodes that are
> parents to pci host bridges. This will make of_dma_configure() to fall
> back to 32-bit size for all devices on all current platforms.  Thus
> applying this patch will immediately break 64-bit dma masks on all
> hardware that supports it.

No, it won't break it, it will just fall back to swiotlb for all the
ones that are lacking the dma-ranges property. I think this is correct
behavior.

> Also related: dma-ranges property used by several pci host bridges is
> *not* compatible with "legacy" dma-ranges parsed by of_get_dma_range() -
> former uses additional flags word at beginning.

Can you elaborate? Do we have PCI host bridges that use wrongly formatted
dma-ranges properties?

Arnd

Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

[Arnd Bergmann]

On Wednesday, January 4, 2017 5:30:19 PM CET Nikita Yushchenko wrote:
> >> For OF platforms, this is called via of_dma_configure(), that checks
> >> dma-ranges of node that is *parent* for host bridge. Host bridge
> >> currently does not control this at all.
> > 
> > We need to think about this a bit. Is it actually the PCI host
> > bridge that limits the ranges here, or the bus that it is connected
> > to. In the latter case, the caller needs to be adapted to handle
> > both.
> 
> In r-car case, I'm not sure what is the source of limitation at physical
> level.
> 
> pcie-rcar driver configures ranges for PCIe inbound transactions based
> on dma-ranges property in it's device tree node. In the current device
> tree for this platform, that only contains one range and it is in lower
> memory.
> 
> NVMe driver tries i/o to kmalloc()ed area. That returns 0x5
> addresses here. As a quick experiment, I tried to add second range to
> pcie-rcar's dma-ranges to cover 0x5 area - but that did not make
> DMA to high addresses working.
> 
> My current understanding is that host bridge hardware module can't
> handle inbound transactions to PCI addresses above 4G - and this
> limitations comes from host bridge itself.
> 
> I've read somewhere in the lists that pcie-rcar hardware is "32-bit" -
> but I don't remember where, and don't know lowlevel details. Maybe
> somebody from linux-renesas can elaborate?

Just a guess, but if the inbound translation windows in the host
bridge are wider than 32-bit, the reason for setting up a single
32-bit window is probably because that is what the parent bus supports.

> >> In current device trees no dma-ranges is defined for nodes that are
> >> parents to pci host bridges. This will make of_dma_configure() to fall
> >> back to 32-bit size for all devices on all current platforms.  Thus
> >> applying this patch will immediately break 64-bit dma masks on all
> >> hardware that supports it.
> > 
> > No, it won't break it, it will just fall back to swiotlb for all the
> > ones that are lacking the dma-ranges property. I think this is correct
> > behavior.
> 
> I'd say - for all ones that have parents without dma-ranges property.
> 
> As of 4.10-rc2, I see only two definitions of wide parent dma-ranges
> under arch/arm64/boot/dts/ - in amd/amd-seattle-soc.dtsi and
> apm/apm-storm.dtsi
> 
> Are these the only arm64 platforms that can to DMA to high addresses?
> I'm not arm64 expert but I'd be surprised if that's the case.

It's likely that a few others also do high DMA, but a lot of arm64
chips are actually derived from earlier 32-bit chips and don't
even support any RAM above 4GB, as well as having a lot of 32-bit
DMA masters.

> >> Also related: dma-ranges property used by several pci host bridges is
> >> *not* compatible with "legacy" dma-ranges parsed by of_get_dma_range() -
> >> former uses additional flags word at beginning.
> > 
> > Can you elaborate? Do we have PCI host bridges that use wrongly formatted
> > dma-ranges properties?
> 
> of_dma_get_range() expects  format.
> 
> pcie-rcar.c, pci-rcar-gen2.c, pci-xgene.c and pcie-iproc.c from
> drivers/pci/host/ all parse dma-ranges using of_pci_range_parser that
> uses  format - i.e. something different
> from what of_dma_get_range() uses.

The "dma_addr" here is expressed in terms of #address-cells of the
bus it is in, and that is "3" in case of PCI, where the first 32-bit
word is a bit pattern containing various things, and the other two
cells are a 64-bit address. I think this is correct, but we may
need to add some special handling for parsing PCI host bridges
in of_dma_get_range, to ensure we actually look at translations for
the memory space.

Arnd

Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

[Arnd Bergmann]

On Tuesday, January 3, 2017 6:44:44 PM CET Will Deacon wrote:
> > @@ -347,6 +348,16 @@ static int __swiotlb_get_sgtable(struct device *dev, 
> > struct sg_table *sgt,
> >  
> >  static int __swiotlb_dma_supported(struct device *hwdev, u64 mask)
> >  {
> > +#ifdef CONFIG_PCI
> > + if (dev_is_pci(hwdev)) {
> > + struct pci_dev *pdev = to_pci_dev(hwdev);
> > + struct pci_host_bridge *br = pci_find_host_bridge(pdev->bus);
> > +
> > + if (br->dev.dma_mask && (*br->dev.dma_mask) &&
> > + (mask & (*br->dev.dma_mask)) != mask)
> > + return 0;
> > + }
> > +#endif
> 
> Hmm, but this makes it look like the problem is both arm64 and swiotlb
> specific, when in reality it's not. Perhaps another hack you could try
> would be to register a PCI bus notifier in the host bridge looking for
> BUS_NOTIFY_BIND_DRIVER, then you could proxy the DMA ops for each child
> device before the driver has probed, but adding a dma_set_mask callback
> to limit the mask to what you need?
> 
> I agree that it would be better if dma_set_mask handled all of this
> transparently, but it's all based on the underlying ops rather than the
> bus type.

This is what I prototyped a long time ago when this first came up.
I still think this needs to be solved properly for all of arm64, not
with a PCI specific hack, and in particular not using notifiers.

Arnd

commit 9a57d58d116800a535510053136c6dd7a9c26e25
Author: Arnd Bergmann <a...@arndb.de>
Date:   Tue Nov 17 14:06:55 2015 +0100

[EXPERIMENTAL] ARM64: check implement dma_set_mask

Needs work for coherent mask

Signed-off-by: Arnd Bergmann <a...@arndb.de>

diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
index 243ef256b8c9..a57e7bb10e71 100644
--- a/arch/arm64/include/asm/device.h
+++ b/arch/arm64/include/asm/device.h
@@ -22,6 +22,7 @@ struct dev_archdata {
void *iommu;/* private IOMMU data */
 #endif
bool dma_coherent;
+   u64 parent_dma_mask;
 };
 
 struct pdev_archdata {
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 290a84f3351f..aa65875c611b 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -352,6 +352,31 @@ static int __swiotlb_dma_supported(struct device *hwdev, 
u64 mask)
return 1;
 }
 
+static int __swiotlb_set_dma_mask(struct device *dev, u64 mask)
+{
+   /* device is not DMA capable */
+   if (!dev->dma_mask)
+   return -EIO;
+
+   /* mask is below swiotlb bounce buffer, so fail */
+   if (!swiotlb_dma_supported(dev, mask))
+   return -EIO;
+
+   /*
+* because of the swiotlb, we can return success for
+* larger masks, but need to ensure that bounce buffers
+* are used above parent_dma_mask, so set that as
+* the effective mask.
+*/
+   if (mask > dev->archdata.parent_dma_mask)
+   mask = dev->archdata.parent_dma_mask;
+
+
+   *dev->dma_mask = mask;
+
+   return 0;
+}
+
 static struct dma_map_ops swiotlb_dma_ops = {
.alloc = __dma_alloc,
.free = __dma_free,
@@ -367,6 +392,7 @@ static struct dma_map_ops swiotlb_dma_ops = {
.sync_sg_for_device = __swiotlb_sync_sg_for_device,
.dma_supported = __swiotlb_dma_supported,
.mapping_error = swiotlb_dma_mapping_error,
+   .set_dma_mask = __swiotlb_set_dma_mask,
 };
 
 static int __init atomic_pool_init(void)
@@ -957,6 +983,18 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, 
u64 size,
if (!dev->archdata.dma_ops)
dev->archdata.dma_ops = _dma_ops;
 
+   /*
+* we don't yet support buses that have a non-zero mapping.
+*  Let's hope we won't need it
+*/
+   WARN_ON(dma_base != 0);
+
+   /*
+* Whatever the parent bus can set. A device must not set
+* a DMA mask larger than this.
+*/
+   dev->archdata.parent_dma_mask = size;
+
dev->archdata.dma_coherent = coherent;
__iommu_setup_dma_ops(dev, dma_base, size, iommu);
 }

Re: [GIT PULL] Renesas ARM Based SoC Fixes for v4.10

[Arnd Bergmann]

On Monday, December 12, 2016 9:30:00 AM CET Simon Horman wrote:
> This provides an sd0_uhs node rather than duplicate sdh0 nodes
> resolving an error introduced in a clean-up patch.
> 
> This pull request is based on "Second Round of Renesas ARM Based SoC DT
> Updates for v4.10", tagged as renesas-arm64-dt2-for-v4.10,
> which you have already pulled. The error corrected by this change
> was introduced in that pull-request.
> 

Pulled into fixes, sorry for the delay.

Arnd

Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

[Arnd Bergmann]

On Friday, January 6, 2017 4:47:59 PM CET Nikita Yushchenko wrote:
> >>> Just a guess, but if the inbound translation windows in the host
> >>> bridge are wider than 32-bit, the reason for setting up a single
> >>> 32-bit window is probably because that is what the parent bus supports.
> 
> I've re-checked rcar-pcie hardware documentation.
> 
> It indeed mentions that AXI bus it sits on is 32-bit.
> 
> 
> >> Well anyway applying patch similar to your's will fix pcie-rcar + nvme
> >> case - thus I don't object :)   But it can break other cases ...
> >>
> >> But why do you hook at set_dma_mask() and overwrite mask inside, instead
> >> of hooking at dma_supported() and rejecting unsupported mask?
> >>
> >> I think later is better, because it lets drivers to handle unsupported
> >> high-dma case, like documented in DMA-API_HOWTO.
> > 
> > I think the behavior I put in there is required for swiotlb to make
> > sense, otherwise you would rely on the driver to handle dma_set_mask()
> > failure gracefully with its own bounce buffers (as network and
> > scsi drivers do but others don't).
> > 
> > Having swiotlb or iommu enabled should result in dma_set_mask() always
> > succeeding unless the mask is too small to cover the swiotlb
> > bounce buffer area or the iommu virtual address space. This behavior
> > is particularly important in case the bus address space is narrower
> > than 32-bit, as we have to guarantee that the fallback to 32-bit
> > DMA always succeeds. There are also a lot of drivers that try to
> > set a 64-bit mask but don't implement bounce buffers for streaming
> > mappings if that fails, and swiotlb is what we use to make those
> > drivers work.
> > 
> > And yes, the API is a horrible mess.
> 
> With my patch applied and thus 32bit dma_mask set for NVMe device, I do
> see high addresses passed to dma_map_*() routines and handled by
> swiotlb. Thus your statement that behavior "succeed 64bit dma_set_mask()
> operation but silently replace mask behind the scene" is required for
> swiotlb to be used, does not match reality.

See my point about drivers that don't implement bounce buffering.
Apparently NVMe is one of them, unlike the SATA/SCSI/MMC storage
drivers that do their own thing.

The problem again is the inconsistency of the API.

> It can be interpreted as a breakage elsewhere, but it's hard to point
> particular "root cause". The entire infrastructure to allocate and use
> DMA memory is messy.

Absolutely.

What I think happened here in chronological order is:

- In the old days, 64-bit architectures tended to use an IOMMU 
  all the time to work around 32-bit limitations on DMA masters
- Some architectures had no IOMMU that fully solved this and the
  dma-mapping API required drivers to set the right mask and check
  the return code. If this failed, the driver needed to use its
  own bounce buffers as network and scsi do. See also the
  grossly misnamed "PCI_DMA_BUS_IS_PHYS" macro.
- As we never had support for bounce buffers in all drivers, and
  early 64-bit Intel machines had no IOMMU, the swiotlb code was
  introduced as a workaround, so we can use the IOMMU case without
  driver specific bounce buffers everywhere
- As most of the important 64-bit architectures (x86, arm64, powerpc)
  now always have either IOMMU or swiotlb enabled, drivers like
  NVMe started relying on it, and no longer handle a dma_set_mask
  failure properly.

We may need to audit how drivers typically handle dma_set_mask()
failure. The NVMe driver in its current state will probably cause
silent data corruption when used on a 64-bit architecture that has
a 32-bit bus but neither swiotlb nor iommu enabled at runtime.

I would argue that the driver should be fixed to either refuse
working in that configuration to avoid data corruption, or that
it should implement bounce buffering like SCSI does. If we make it
simply not work, then your suggestion of making dma_set_mask()
fail will break your system in a different way.

> Still current code does not work, thus fix is needed.
> 
> Perhaps need to introduce some generic API to "allocate memory best
> suited for DMA to particular device", and fix allocation points (in
> drivers, filesystems, etc) to use it. Such an API could try to allocate
> area that can be DMAed by hardware, and fallback to other memory that
> can be used via swiotlb or other bounce buffer implementation.

The DMA mapping API is meant to do this, but we can definitely improve
it or clarify some of the rules.
 
> But for now, have to stay with dma masks. Will follow-up with a patch
> based on your but with coherent mask handling added.

Ok.

Arnd

Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

[Arnd Bergmann]

On Tuesday, January 10, 2017 3:47:25 PM CET Nikita Yushchenko wrote:
> 
> > The point here is that an IOMMU doesn't solve your issue, and the
> > IOMMU-backed DMA ops need the same treatment. In light of that, it really
> > feels to me like the DMA masks should be restricted in of_dma_configure
> > so that the parent mask is taken into account there, rather than hook
> > into each set of DMA ops to intercept set_dma_mask.

of_dma_configure() sets up a 32-bit mask, which is assumed to always work
in the kernel. We can't change it to a larger mask because that would
break drivers that have to restrict devices to 32-bit.

If the bus addressing is narrower than 32 bits however, the initial mask
should probably be limited to whatever the bus supports, but that is not
the problem we are trying to solve here.

> > We'd still need to
> > do something to stop dma_set_mask widening the mask if it was restricted
> > by of_dma_configure, but I think Robin (cc'd) was playing with that.
> 
> What issue "IOMMU doesn't solve"?
> 
> Issue I'm trying to address is - inconsistency within swiotlb
> dma_map_ops, where (1) any wide mask is silently accepted, but (2) then
> mask is used to decide if bounce buffers are needed or not. This
> inconsistency causes NVMe+R-Car cobmo not working (and breaking memory
> instead).

It's not just an inconsistency, it's a known bug that we really
need to fix.

> I just can't think out what similar issue iommu can have.
> Do you mean that in iommu case, mask also must not be set to whatever
> wider than initial value? Why? What is the use of mask in iommu case? Is
> there any real case when iommu can't address all memory existing in the
> system?

I think the problem that Will is referring to is when the IOMMU has
a virtual address space that is wider than the DMA mask of the device:
In this case, dma_map_single() might return a dma_addr_t that is not
reachable by the device.

I'd consider that a separate bug that needs to be worked around in
the IOMMU code.

> NVMe maintainer has just stated that they expect
> set_dma_mask(DMA_BIT_MASK(64)) to always succeed, and are going to error
> out driver probe if that call fails.  They claim that architecture must
> always be able to dma_map() whatever memory existing in the system - via
> iommu or swiotlb or whatever. Their direction is to remove bounce
> buffers from block and other layers.
> 
> With this direction, semantics of dma mask becomes even more
> questionable. I'd say dma_mask is candidate for removal (or to move to
> swiotlb's or iommu's local area)

Removing dma_mask is not realistic any time soon, there are too many things
in the kernel that use it for one thing or another, so any changes here
have to be done really carefully. We definitely need the mask to support
architectures without swiotlb.

Arnd

Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

[Arnd Bergmann]

On Monday, January 9, 2017 9:57:46 PM CET Christoph Hellwig wrote:
> > - architecture should stop breaking 64-bit DMA when driver attempts to
> > set 64-bit dma mask,
> > 
> > - NVMe should issue proper blk_queue_bounce_limit() call based on what
> > is actually set mask,
> 
> Or even better remove the call to dma_set_mask_and_coherent with
> DMA_BIT_MASK(32).  NVMe is designed around having proper 64-bit DMA
> addressing, there is not point in trying to pretent it works without that

Agreed, let's just fail the probe() if DMA_BIT_MASK(64) fails, and
have swiotlb work around machines that for some reason need bounce
buffers.

> > - and blk_queue_bounce_limit() should also be fixed to actually set
> > 0x limit, instead of replacing it with (max_low_pfn <<
> > PAGE_SHIFT) as it does now.
> 
> We need to kill off BLK_BOUNCE_HIGH, it just doesn't make sense to
> mix the highmem aspect with the addressing limits.  In fact the whole
> block bouncing scheme doesn't make much sense at all these days, we
> should rely on swiotlb instead.

If we do this, we should probably have another look at the respective
NETIF_F_HIGHDMA support in the network stack, which does the same thing
and mixes up highmem on 32-bit architectures with the DMA address limit.
(side note: there are actually cases in which you have a 31-bit DMA
mask but 3 GB of lowmem using CONFIG_VMSPLIT_1G, so BLK_BOUNCE_HIGH
and !NETIF_F_HIGHDMA are both missing the limit, causing data corruption
without swiotlb).

Before we rely too much on swiotlb, we may also need to consider which
architectures today rely on bouncing in blk and network.

I see that we have CONFIG_ARCH_PHYS_ADDR_T_64BIT on a couple of
32-bit architectures without swiotlb (arc, arm, some mips32), and
there are several 64-bit architectures that do not have swiotlb
(alpha, parisc, s390, sparc). I believe that alpha, s390 and sparc
always use some form of IOMMU, but the other four apparently don't,
so we would need to add swiotlb support there to remove all the
bounce buffering in network and block layers.

Arnd

Re: NVMe vs DMA addressing limitations

[Arnd Bergmann]

On Tuesday, January 10, 2017 8:07:20 AM CET Christoph Hellwig wrote:
> On Tue, Jan 10, 2017 at 09:47:21AM +0300, Nikita Yushchenko wrote:
> > I'm now working with HW that:
> > - is now way "low end" or "obsolete", it has 4G of RAM and 8 CPU cores,
> > and is being manufactured and developed,
> > - has 75% of it's RAM located beyond first 4G of address space,
> > - can't physically handle incoming PCIe transactions addressed to memory
> > beyond 4G.
> 
> It might not be low end or obselete, but it's absolutely braindead.
> Your I/O performance will suffer badly for the life of the platform
> because someone tries to save 2 cents, and there is not much we can do
> about it.

Unfortunately it is a common problem for arm64 chips that were designed
by taking a 32-bit SoC and replacing the CPU core. The swiotlb is the
right workaround for this, and I think we all agree that we should
just make it work correctly.

Arnd

Re: NVMe vs DMA addressing limitations

[Arnd Bergmann]

On Tuesday, January 10, 2017 10:31:47 AM CET Nikita Yushchenko wrote:
> Christoph, thanks for clear input.
> 
> Arnd, I think that given this discussion, best short-term solution is
> indeed the patch I've submitted yesterday. That is, your version +
> coherent mask support.  With that, set_dma_mask(DMA_BIT_MASK(64)) will
> succeed and hardware with work with swiotlb.

Ok, good.

> Possible next step is to teach swiotlb to dynamically allocate bounce
> buffers within entire arm64's ZONE_DMA.

That seems reasonable, yes. We probably have to do both, as there are
cases where a device has dma_mask smaller than ZONE_DMA but the swiotlb
bounce area is low enough to work anyway.

Another workaround me might need is to limit amount of concurrent DMA
in the NVMe driver based on some platform quirk. The way that NVMe works,
it can have very large amounts of data that is concurrently mapped into
the device. SWIOTLB is one case where this currently fails, but another
example would be old PowerPC servers that have a 256MB window of virtual
I/O addresses per VM guest in their IOMMU. Those will likely fail the same
way that your does.

> Also there is some hope that R-Car *can* iommu-translate addresses that
> PCIe module issues to system bus.  Although previous attempts to make
> that working failed. Additional research is needed here.

Does this IOMMU support remapping data within a virtual machine? I believe
there are some that only do one of the two -- either you can have guest
machines with DMA access to their low memory, or you can remap data on
the fly in the host.

Arnd

Re: [PATCH v2 0/2] base: soc: soc_device_match() improvements

[Arnd Bergmann]

On Wed, Mar 29, 2017 at 9:38 PM, Geert Uytterhoeven
 wrote:
> Hi Arnd, Greg, Kevin, Magnus, Olof, Simon,
>
> This patch series contains two improvements for the SoC bus and
> soc_device_match().  The second one is a dependency for handling
> different SoC revisions in the Renesas R-Car SYSC driver, which manages
> PM Domains and thus needs to be initialized from an early_initcall[*].
>
> Changes compared to v1:
>   - Drop RFC state,
>   - Add more explanation,
>   - Add Acked-by.
>
> Due to the dependency, and as changes for the Renesas R-Car SYSC driver
> typically go in through the renesas and arm-soc trees, and if you agree,
> I would like to send a pull request for these changes, to be pulled by
> all of the driver core, renesas, and arm-soc trees.  This is similar to
> how soc_device_match() was integrated earlier.
> Then the Renesas R-Car SYSC driver changes can easily be queued on top
> in the renesas tree afterwards.
>
> Do you agree?
> Thanks for your answer!

Merging this through arm-soc is fine with me.

> P.S. If you think it's already too late in the cycle to queue Renesas
> R-Car SYSC driver changes for v4.12 on top, please say so.  Then the
> dependency is relaxed, and this series can just go in through the
> driver core tree, with the Renesas part to follow for v4.13.

We can take it for 4.12.

  Arnd

Re: [PATCH 1/2] [RFC] base: soc: Let soc_device_match() return no match when called too early

[Arnd Bergmann]

On Thu, Mar 9, 2017 at 7:18 PM, Geert Uytterhoeven
<geert+rene...@glider.be> wrote:
> If soc_device_match() is called before the SoC bus has been registered,
> bus_for_each_dev() returns -EINVAL, which is considered a match, as it
> is non-zero.
>
> While calling soc_device_match() can be considered an integration
> mistake, returning a match is counter-intuitive: soc_device_match() is
> typically used to handle quirks, i.e. to deviate from the default path.
> Hence add a check to abort checking and return no match instead.
>
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>
> ---

Acked-by: Arnd Bergmann <a...@arndb.de>

> Should this use WARN_ONCE()?

I wouldn't mind that either, but your current version seems sufficient.

Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single SoC device

[Arnd Bergmann]

On Mon, Mar 13, 2017 at 1:46 PM, Geert Uytterhoeven
<ge...@linux-m68k.org> wrote:
> On Mon, Mar 13, 2017 at 1:41 PM, Arnd Bergmann <a...@arndb.de> wrote:
>> On Thu, Mar 9, 2017 at 7:18 PM, Geert Uytterhoeven
>>
>> I'd prefer to not have to do the early registration at all and have fewer
>> special cases. Can you list a specific example that requires this?
>
> The specific example is the Renesas R-Car SYSC driver, which manages PM
> Domains and thus needs to be initialized from an early_initcall.

Ok, and what prevents us from using information in DT to detect which
variant we have? Is this a case of absolutely having to know the exact
hardware revision at the time of initialization, or is it just to simplify the
implementation of the SYSC driver?

 Arnd

Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single SoC device

[Arnd Bergmann]

On Thu, Mar 9, 2017 at 7:18 PM, Geert Uytterhoeven
 wrote:
> commit 1da1b3628df34a2a ("base: soc: Early register bus when needed")
> added support for early registration of SoC devices from a
> core_initcall().  However, some drivers need to check the SoC revision
> from an early_initcall(), which is even earlier.
>
> While registering the SoC bus and device, and using soc_device_match(),
> from an early_initcall() do work, the "soc" directory and the "soc0"
> file end up wrongly in the sysfs root, as the "bus" resp. "devices"
> directories haven't been created yet.
>
> To fix this, allow to register a single SoC device early on.
> As long as the SoC bus isn't registered, soc_device_match() just
> matches against this early device.
> When the SoC bus is registered later, the early device is registered for
> real.
>
> Note that soc_device_register() returns NULL (no error, but also not a
> valid pointer) when registering an early device.  Hence platform devices
> cannot be instantiated as children of the "soc0" node representing an
> early SoC device.  This should not be an issue, as that practice has
> been deprecated for new platforms.
>
> Signed-off-by: Geert Uytterhoeven 
> ---
> Is there a better way to fix this?
> While I believe all current users register a single SoC device, we may
> want to register multiple SoC devices in the future?
>
> soc_device_match() itself is completely independent from sysfs, but the
> sysfs operations are tied intimately to the driver core.
>
> The "bus" and "devices" directories are created from do_basic_setup() ->
> driver_init() -> devices_init() / buses_init(), which runs in between
> do_pre_smp_initcalls() (early_initcall()) and do_initcalls().

I'd prefer to not have to do the early registration at all and have fewer
special cases. Can you list a specific example that requires this?

If we want to do early registration, then your implementation seems
fine to me.

 Arnd

Re: [GIT PULL] Third Round of Renesas ARM Based SoC Fixes for v4.13

[Arnd Bergmann]

On Mon, Jul 31, 2017 at 1:12 PM, Simon Horman
 wrote:
> Hi Olof, Hi Kevin, Hi Arnd,
>
> Please consider these third round of Renesas ARM based SoC fixes for v4.13.
>
> This pull request is based on v4.12-rc1 rather than earlier fixes for v4.13
> as it is unrelated to those fixes and their dependencies - arm64 DT updates
> for R-Car Gen 3 SoCs.
>
> This fix resolves a deadlock during boot on R-Car Gen 2 SoCs.
> The bug has been present since v4.1-rc1 but does not manifest in
> a deadlock until v4.13-rc1.
>

Pulled into fixes, thanks!

   Arnd

Re: [GIT PULL] Renesas ARM Based SoC Defconfig Updates for v4.14

[Arnd Bergmann]

On Mon, Jul 31, 2017 at 5:03 PM, Simon Horman
 wrote:

> 
> Renesas ARM Based SoC Defconfig Updates for v4.14
>
> * Enable DMA for Renesas serial ports
>
>   Geert Uytterhoeven says, "DMA for (H)SCIF(A|B) serial ports on Renesas
>   R-Car Gen2 and RZ/G1 SoCs is considered stable, hence enable it by
>   default.".
>
> * Enable Ethernet AVB
>
>   For the iWave RZ/G1M Q7 SOM
>
> * Replace DRM_RCAR_HDMI by generic bridge options
> * Replace SND_SOC_RSRC_CARD by SND_SIMPLE_SCU_CARD
> * Replace USB_XHCI_RCAR by USB_XHCI_PLATFORM
> * Enable missing PCIE_RCAR dependency
>
>   Defconfig updates for various Kconfig updates covering
>   renamed Kconfig symbols, now missing dependancies and so on.

Pulled into next/defconfig, thanks!

Arnd

Re: [GIT PULL] Renesas ARM64 Based SoC Defconfig Updates for v4.14

[Arnd Bergmann]

On Mon, Jul 31, 2017 at 5:02 PM, Simon Horman
 wrote:

> 
> Renesas ARM64 Based SoC Defconfig Updates for v4.14
>
> * compile ak4613 and renesas sound as modules
>
>   This is intended to reduce the size of a kernel image compiled
>   using the defconfig. This is timely as it brings the kernel image
>   back below the size that can be booted in my environment, a limit
>   it crept over in v4.13-rc1.

Good idea, thanks! Pulled into next/arm64.

   Arnd

Re: [GIT PULL] Fourth Round of Renesas ARM Based SoC Fixes for v4.13

[Arnd Bergmann]

On Thu, Aug 17, 2017 at 10:56 AM, Simon Horman
 wrote:
> Hi Olof, Hi Kevin, Hi Arnd,
>
> Please consider these fourth round of Renesas ARM based SoC fixes for v4.13.
>
> This pull request is based on the second round of
> such requests, tagged as renesas-fixes2-for-v4.13,
> which I you have already pulled.
>
> It is not based on the third round of fixes as they
> are not related to arm64 DT and have a different base.

Pulled into the fixes branch, thanks!

   Arnd

Re: [GIT PULL] Renesas ARM Based SoC DT Bindings Updates for v4.14

[Arnd Bergmann]

On Mon, Jul 31, 2017 at 5:04 PM, Simon Horman
 wrote:
> Renesas ARM Based SoC DT Bindings Updates for v4.14
>
> * Document R-Car D3 (r8a77995) SoC and Draak board
>
> * Document reserved SRAM for the SMP jump stub on R-Car Gen2 and RZ/G1 SoCs
>
>   Geert Uytterhoeven says, "+Renesas R-Car Gen2 and RZ/G1 SoCs need a small
>   piece of SRAM for the jump stub +for secondary CPU bringup and CPU
>   hotplug.  +This memory is reserved by adding a child node to a
>   "mmio-sram" node..."
>
> * Add a VSP channel index to DU vsps
>
>   Laurent Pinchart says, "multiple LIF instances in the VSP. The current DT
>   bindings don't support specifying that kind of SoC integration scheme.
>   Extend them with a VSP channel index."
>
> * Add R-Car M3-W (r8a7796) HDMI TX DT bindings
>
>   This is compatible with existing R-Car H3 (r8a7795) support

Pulled into next/dt, thanks!

   Arnd

Re: [GIT PULL] Renesas ARM64 Based SoC DT Updates for v4.14

[Arnd Bergmann]

On Mon, Jul 31, 2017 at 5:03 PM, Simon Horman
 wrote:
> 
> Renesas ARM64 Based SoC DT Updates for v4.14
>
> * Add usb2.0 for R-Car H3 (r8a7795) ES2.0 SoC
>
> * Add R-Car D3 (r8a77995) SoC and Draak board support
>
>   Adds minimal support for the R-Car D3 SoC and the Draak development
>   board, allowing to boot from a ramdisk using a serial console.
>
> * Add Add VC6 clock generator to R-Car H3 (r8a7795)/Salvator-XS board
>
>   The VC6 is an I2C-controlled programmable clock generator, used on the
>   board to provide a display dot clock. Add it to DT.
>
> * Add missing second pair of DMA names to MSIOF nodes to
>   R-Car M3-W (r8a7796) SoC
>
>   MSIOF0 and MSIOF1 are tied to two DMA controllers through two pairs of
>   DMA specifiers.  However, the second pair of corresponding DMA names was
>   missing.
>
> * Add support for the DU to R-Car H3 (r8a7795) SoC
>
>   Add a compatible string and VSP links to the DU node. The H3 ES1.x and H3
>   ES2.0 are compatible save for the links to the VSPs that are described
>   explicitly in DT, so there's no need for a new ES2-specific compatible
>   string.
>
> * Enable HDMI on R-Car H3 (r8a7795) and M3-W (r8a7796) ULCB boards
>
> * Enable DU on R-Car M3-W (r8a7796) Salvator-X board
>
> * Enable I2C for DVFS on R-Car H3 (r8a7795) and M3-W (r8a7796) ULCB boards
>
> * Add Add DRIF support to R-Car H3 (r8a7795) and M3-W (r8a7796) SoCs
>
>   Ramesh Shanmugasundaram says, "R-Car Gen3 DRIF is a SPI like receive only
>   slave device."
>
> * Move CPG_AUDIO_CLK_I from board to soc files
>
>   Geert Uytterhoeven says, "The definition of CPG_AUDIO_CLK_I is
>   SoC-specific, not board-specific."
>
> * Add IMR-LX4 support to R-Car H3 (r8a7795) and M3-W (r8a7796) SoCs
>
>   Sergei Shtylyov says, "The image renderer light extended 4 (IMR-LX4) or
>   the distortion correction engine is a drawing processor with a simple
>   instruction system capable of referencing data on an external memory as
>   2D texture data and performing texture mapping and drawing with respect
>   to any shape that is split into triangular objects."

Pulled into next/dt64, thanks!

   Arnd

Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

[Arnd Bergmann]

On Mon, Jul 17, 2017 at 3:45 PM, Hans Verkuil <hverk...@xs4all.nl> wrote:
> On 14/07/17 11:36, Arnd Bergmann wrote:
>> @@ -201,8 +202,9 @@ static int cx18_g_fmt_sliced_vbi_cap(struct file *file, 
>> void *fh,
>>* digitizer/slicer.  Note, cx18_av_vbi() wipes the passed in
>>* fmt->fmt.sliced under valid calling conditions
>>*/
>> - if (v4l2_subdev_call(cx->sd_av, vbi, g_sliced_fmt, >fmt.sliced))
>> - return -EINVAL;
>> + ret = v4l2_subdev_call(cx->sd_av, vbi, g_sliced_fmt, >fmt.sliced);
>> + if (ret)
>> + return ret;
>
> Please keep the -EINVAL here. I can't be 100% certain that returning 'ret' 
> wouldn't
> break something.

I think Dan was recommending the opposite here, if I understood you
both correctly:
he said we should propagate the error code unless we know it's wrong, while you
want to keep the current behavior to avoid introducing changes ;-)

I guess in either case, looking at the callers more carefully would be
a good idea.

>> - return 0;
>> + return ret;
>>  }
>>
>>  int atomisp_flash_enable(struct atomisp_sub_device *asd, int num_frames)
>>
>
> This is all very hackish, though. I'm not terribly keen on this patch. It's 
> not
> clear to me *why* these warnings appear in your setup.

it's possible that this only happened with 'ccache', which first preprocesses
the source and the passes it with v4l2_subdev_call expanded into the
compiler. This means the line looks like

if ((!(cx->sd_av) ? -ENODEV :
(((cx->sd_av)->ops->vbi && (cx->sd_av)->ops->vbi->g_sliced_fmt) ?
   (cx->sd_av)->ops->vbi->g_sliced_fmt(cx->sd_av)),
>fmt.sliced) :
   -ENOIOCTLCMD))

The compiler now complains about the sub-expression that it sees for
cx->sd_av==NULL:

   if (-ENODEV)

which it considers nonsense because it is always true and the value gets
ignored.

Let me try again without ccache for now and see what warnings remain.
We can find a solution for those first, and then decide how to deal with
ccache.

Arnd

Re: [PATCH 9/9] ARM: multi_v7_defconfig: Enable DMA for Renesas serial ports

[Arnd Bergmann]

On Tue, Jul 11, 2017 at 8:59 AM, Geert Uytterhoeven
 wrote:

> Whether we want to hide the option (and default to y if ARCH_RENESAS, i.e.
> ARM) is another question.  Note that that wouldn't reduce code 
> maintainability,
> as the #ifdefs would be kept, and it would prevent building and testing
> with/without DMA on ARM.

arch/sh/configs/sdk7786_defconfig enables it, too, so it should at least be
a user-selectable option on SuperH if you do that.

 Arnd

Re: [PATCH 9/9] ARM: multi_v7_defconfig: Enable DMA for Renesas serial ports

[Arnd Bergmann]

On Tue, Jul 11, 2017 at 8:59 AM, Geert Uytterhoeven
 wrote:
> Hi Magnus,
>
> On Tue, Jul 11, 2017 at 5:38 AM, Magnus Damm  wrote:
>> On Mon, Jul 10, 2017 at 10:28 PM, Geert Uytterhoeven

>> Since enabling DMA Engine still keeps PIO support around I wonder why
>> we need this Kconfig at all - other drivers seem to get by without
>> this kind of thing?
>> So in my opinion it would also be nice to get rid of SERIAL_SH_SCI_DMA
>> completely and reducing the number of special per-driver Kconfig
>> entries.
>
> In general, I would agree, and remove the option at the blimp of an eye.
> However, this driver is shared with H8/300 and SuperH.  While both could use
> DMA (but it's not supported by Linux yet), I don't know if they are willing to
> live with the increased static and dynamic memory footprint of SH_SCI DMA
> support.

One more thing: enabling the DMA support in the console driver generally
means you cannot use printk in the DMA driver anywhere that may be called
during printk(). Not sure if that is a concern here.

  Arnd

Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

[Arnd Bergmann]

On Fri, Jul 14, 2017 at 3:09 PM, Dan Carpenter  wrote:
> On Fri, Jul 14, 2017 at 03:55:26PM +0300, Dan Carpenter wrote:
>> I don't agree with it as a static analysis dev...
>
> What I mean is if it's a macro that returns -ENODEV or a function that
> returns -ENODEV, they should both be treated the same.  The other
> warnings this check prints are quite clever.

I think this is what gcc tries to do, and it should work normally, but it
fails when using ccache. I know I had cases like that, not entirely sure
if this is one of them. Maybe it just means I should give up on using
ccache in preprocessor mode.

   Arnd