Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi
From: Mark Brown @ 2018-05-24 18:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <0e6e71e2-f8ac-7889-0d81-8d8a4c15223d@microchip.com>

On Thu, May 24, 2018 at 07:04:11PM +0300, Radu Pirea wrote:

>     if (ctlr->cs_gpios){
>         spi->cs_gpio = ctlr->cs_gpios[spi->chip_select];
>         if(gpio_is_valid(spi->cs_gpio))
>             gpio_direction_output(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
> 
>     }

You're expected to request the GPIOs in your driver using one of the
modern request functions like gpio_request_one() (or ideally the GPIO
descriptor APIs now) which combine the direction setting and request
into a single operation.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/afe4d42a/attachment.sig>

^ permalink raw reply

* [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi
From: Mark Brown @ 2018-05-24 18:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524175620.GB12093@piout.net>

On Thu, May 24, 2018 at 07:56:20PM +0200, Alexandre Belloni wrote:

> Back in 2014, I was suggesting using devm_gpio_request_one() in
> of_spi_register_master(). That would take care of setting the direction
> of the GPIO:

> https://www.spinics.net/lists/arm-kernel/msg351251.html

> I never took the time to create the patch and test though.

Right, this'd be ideal but unfortunately it does mean we'd have to
transition all the existing users that open code their requests which is
a pain.  It'd be really nice if someone had the time/enthusiam to do it
though.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/4bbd58ed/attachment.sig>

^ permalink raw reply

* [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Mark Brown @ 2018-05-24 18:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524175024.19874-2-robh@kernel.org>

On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:

> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.

Should userspace have some involvement in this decision?  It knows if
it's got any intention of loading modules for example.  Kernel config
checks might be good enough, though it's going to be a pain to work out
if the relevant driver is built as a module for example.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/252c8b9e/attachment.sig>

^ permalink raw reply

* [GIT PULL 1/2] ARM: dts: exynos: Second round for v4.18
From: Krzysztof Kozlowski @ 2018-05-24 18:20 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On top of previous pull request.

Best regards,
Krzysztof


The following changes since commit 83cb529b2ef4f3446e60e75522d76fdaaea4724c:

  ARM: dts: exynos: Update x and y properties for mms114 touchscreen (2018-05-13 15:15:49 +0200)

are available in the git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git tags/samsung-dt-4.18-2

for you to fetch changes up to 68605101460ea4c62a966b1ad3e8db90d8fbaa31:

  ARM: dts: exynos: Add support for audio over HDMI for Odroid X/X2/U3 (2018-05-15 18:46:14 +0200)

----------------------------------------------------------------
Samsung DTS ARM changes for v4.18, part 2

1. Add support for audio over HDMI for Odroid X/X2/U3.

----------------------------------------------------------------
Sylwester Nawrocki (1):
      ARM: dts: exynos: Add support for audio over HDMI for Odroid X/X2/U3

 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 33 +++++++++++++++----------
 arch/arm/boot/dts/exynos4412-odroidu3.dts       |  6 ++---
 arch/arm/boot/dts/exynos4412-odroidx.dts        |  6 ++---
 3 files changed, 26 insertions(+), 19 deletions(-)

^ permalink raw reply

* [GIT PULL 2/2] arm64: dts: exynos: Second round for v4.18
From: Krzysztof Kozlowski @ 2018-05-24 18:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524182016.5866-1-krzk@kernel.org>

Hi,

On top of previous pull request.

Best regards,
Krzysztof


The following changes since commit 8dd6203f32f20cb83469eb859efded9e403b3e9f:

  arm64: dts: exynos: Add mem-2-mem Scaler devices (2018-05-13 11:26:13 +0200)

are available in the git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git tags/samsung-dt64-4.18-2

for you to fetch changes up to f0b5e8a21e6604980c35eeeba1ee3a124f45ad1f:

  arm64: dts: exynos: Add more clocks to Exynos5433 Decon/DeconTV (2018-05-23 20:23:24 +0200)

----------------------------------------------------------------
Samsung DTS ARM64 changes for v4.18, part 2

1. Add clocks necessary for DECON hardware windows no 4 and 5 on
   Exynos5433.

----------------------------------------------------------------
Marek Szyprowski (1):
      arm64: dts: exynos: Add more clocks to Exynos5433 Decon/DeconTV

 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

^ permalink raw reply

* [PATCH V2] PCI/portdrv: do not disable device on reboot/shutdown
From: Bjorn Helgaas @ 2018-05-24 18:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <61f70fd6-52fd-da07-ce73-303f95132131@codeaurora.org>

On Wed, May 23, 2018 at 06:57:18PM -0400, Sinan Kaya wrote:
> On 5/23/2018 5:32 PM, Bjorn Helgaas wrote:
> > 
> > The crash seems to indicate that the hpsa device attempted a DMA after
> > we cleared the Root Port's PCI_COMMAND_MASTER, which means
> > hpsa_shutdown() didn't stop DMA from the device (it looks like *most*
> > shutdown methods don't disable device DMA, so it's in good company).
> 
> All drivers are expected to shutdown DMA and interrupts in their shutdown()
> routines. They can skip removing threads, data structures etc. but DMA and
> interrupt disabling are required. This is the difference between shutdown()
> and remove() callbacks.
> 
> If you see that this is not being done in HPSA, then that is where the
> bugfix should be.
> 
> Counter argument is that if shutdown() is not implemented, at least remove()
> should be called. Expecting all drivers to implement shutdown() callbacks
> is just bad by design in my opinion. 
> 
> Code should have fallen back to remove() if shutdown() doesn't exist.
> I can propose a patch for this but this is yet another story to chase.

That sounds like a reasonable idea, and it is definitely another can
of worms.  I looked briefly at some of the .shutdown() cases:

  - device_shutdown() doesn't fall back to remove().

  - It looks like most bus_types don't implement .shutdown() at all (I
    didn't look at them all).

  - Of the bus_types that do implement .shutdown(), most do not fall
    back to .remove().  ps3_system_bus_type() is an example of one
    that *does* fall back to a driver's .remove() if there is no
    .shutdown().

Implement shutdown (no fallback unless indicated):

  ecard_bus_type
  gio_bus_type
  ps3_system_bus_type	# does fallback to remove
  ibmebus_bus_type
  isa_bus_type
  platform_bus_type	# not direct implementation
  fmc_bus_type		# fmc_shutdown() looks spurious
  mipi_dsi_bus_type
  hv_bus

> >> This has been found to cause crashes on HP DL360 Gen9 machines during
> >> reboot. Besides, kexec is already clearing the bus master bit in
> >> pci_device_shutdown() after all PCI drivers are removed.
> > 
> > The original path was:
> > 
> >   pci_device_shutdown(hpsa)
> >     drv->shutdown
> >       hpsa_shutdown                     # hpsa_pci_driver.shutdown
> >   ...
> >   pci_device_shutdown(RP)               # root port
> >     drv->shutdown
> >       pcie_portdrv_remove               # pcie_portdriver.shutdown
> >         pcie_port_device_remove
> >           pci_disable_device
> >             do_pci_disable_device
> >               # clear RP PCI_COMMAND_MASTER
> >     if (kexec)
> >       pci_clear_master(RP)
> >         # clear RP PCI_COMMAND_MASTER
> > 
> > If I understand correctly, the new path after this patch is:
> > 
> >   pci_device_shutdown(hpsa)
> >     drv->shutdown
> >       hpsa_shutdown                     # hpsa_pci_driver.shutdown
> >   ...
> >   pci_device_shutdown(RP)               # root port
> >     drv->shutdown
> >       pcie_portdrv_shutdown             # pcie_portdriver.shutdown
> >         __pcie_portdrv_remove(RP, false)
> >           pcie_port_device_remove(RP, false)
> >             # do NOT clear RP PCI_COMMAND_MASTER
> 
> yup
> 
> >     if (kexec)
> >       pci_clear_master(RP)
> >         # clear RP PCI_COMMAND_MASTER
> > 
> > I guess this patch avoids the panic during reboot because we're not in
> > the kexec path, so we never clear PCI_COMMAND_MASTER for the Root
> > Port, so the hpsa device can DMA happily until the lights go out.
> > 
> > But DMA continuing for some random amount of time before the reboot or
> > shutdown happens makes me a little queasy.  That doesn't sound safe.
> > The more I think about this, the more confused I get.  What am I
> > missing?  
> 
> see above.
> 
> > 
> >> Just remove the extra clear in shutdown path by seperating the remove and
> >> shutdown APIs in the PORTDRV.
> >>
> >>  static pci_ers_result_t pcie_portdrv_error_detected(struct pci_dev *dev,
> >> @@ -218,7 +228,7 @@ static struct pci_driver pcie_portdriver = {
> >>  
> >>  	.probe		= pcie_portdrv_probe,
> >>  	.remove		= pcie_portdrv_remove,
> >> -	.shutdown	= pcie_portdrv_remove,
> >> +	.shutdown	= pcie_portdrv_shutdown,
> > 
> > What are the circumstances when we call .remove() vs .shutdown()?
> > 
> > I guess the main (maybe only) way to call .remove() is to hot-remove
> > the port?  And .shutdown() is basically used in the reboot and kexec
> > paths?
> 
> Correct. shutdown() is only called during reboot/shutdown calls. If you echo
> 1 into the remove file, remove() gets called. Handy for hotplug use cases.
> It needs to be the exact opposite of the probe. It needs to clean up resources
> etc. and have the HW in a state where it can be reinitialized via probe again.
> 
> > 
> >>  	.err_handler	= &pcie_portdrv_err_handler,
> >>  
> >> -- 
> >> 2.7.4
> >>
> > 
> 
> 
> -- 
> Sinan Kaya
> Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
> Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply

* [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Greg Kroah-Hartman @ 2018-05-24 18:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524175024.19874-2-robh@kernel.org>

On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
> Deferred probe will currently wait forever on dependent devices to probe,
> but sometimes a driver will never exist. It's also not always critical for
> a driver to exist. Platforms can rely on default configuration from the
> bootloader or reset defaults for things such as pinctrl and power domains.
> This is often the case with initial platform support until various drivers
> get enabled. There's at least 2 scenarios where deferred probe can render
> a platform broken. Both involve using a DT which has more devices and
> dependencies than the kernel supports. The 1st case is a driver may be
> disabled in the kernel config. The 2nd case is the kernel version may
> simply not have the dependent driver. This can happen if using a newer DT
> (provided by firmware perhaps) with a stable kernel version.
> 
> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.
> 
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/base/dd.c      | 17 +++++++++++++++++
>  include/linux/device.h |  2 ++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index c9f54089429b..d6034718da6f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>  	driver_deferred_probe_trigger();
>  }
>  
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
> +{
> +	if (optional && initcalls_done) {
> +		dev_WARN(dev, "ignoring dependency for device, assuming no driver");

You really only need dev_warn(), here, right?

thanks,

greg k-h

^ permalink raw reply

* [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Greg Kroah-Hartman @ 2018-05-24 19:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524175024.19874-2-robh@kernel.org>

On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
> Deferred probe will currently wait forever on dependent devices to probe,
> but sometimes a driver will never exist. It's also not always critical for
> a driver to exist. Platforms can rely on default configuration from the
> bootloader or reset defaults for things such as pinctrl and power domains.
> This is often the case with initial platform support until various drivers
> get enabled. There's at least 2 scenarios where deferred probe can render
> a platform broken. Both involve using a DT which has more devices and
> dependencies than the kernel supports. The 1st case is a driver may be
> disabled in the kernel config. The 2nd case is the kernel version may
> simply not have the dependent driver. This can happen if using a newer DT
> (provided by firmware perhaps) with a stable kernel version.
> 
> Subsystems or drivers may opt-in to this behavior by calling
> driver_deferred_probe_check_init_done() instead of just returning
> -EPROBE_DEFER. They may use additional information from DT or kernel's
> config to decide whether to continue to defer probe or not.
> 
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/base/dd.c      | 17 +++++++++++++++++
>  include/linux/device.h |  2 ++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index c9f54089429b..d6034718da6f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>  	driver_deferred_probe_trigger();
>  }
>  
> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
> +{
> +	if (optional && initcalls_done) {

Wait, what's the "optional" mess here?

The caller knows this value, so why do you need to even pass it in here?

And bool values that are not obvious are horrid.  I had to go look this
up when reading the later patches that just passed "true" in this
variable as I had no idea what that meant.

So as-is, no, this isn't ok, sorry.

And at the least, this needs some kerneldoc to explain it :)

thanks,

greg k-h

^ permalink raw reply

* [PATCH v2 2/8] driver core: add a deferred probe timeout
From: Greg Kroah-Hartman @ 2018-05-24 19:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524175024.19874-3-robh@kernel.org>

On Thu, May 24, 2018 at 12:50:18PM -0500, Rob Herring wrote:
> Deferring probe can wait forever on dependencies that may never appear
> for a variety of reasons. This can be difficult to debug especially if
> the console has dependencies or userspace fails to boot to a shell. Add
> a timeout to retry probing without possibly optional dependencies and to
> dump out the deferred probe pending list after retrying.
> 
> This mechanism is intended for debug purposes. It won't work for the
> console which needs to be enabled before userspace starts. However, if
> the console's dependencies are resolved, then the kernel log will be
> printed (as opposed to no output).
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  .../admin-guide/kernel-parameters.txt         |  7 +++++
>  drivers/base/dd.c                             | 28 ++++++++++++++++++-
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 11fc28ecdb6d..dd3f40b34a24 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -809,6 +809,13 @@
>  			Defaults to the default architecture's huge page size
>  			if not specified.
>  
> +	deferred_probe_timeout=
> +			[KNL] Set a timeout in seconds for deferred probe to
> +			give up waiting on dependencies to probe. Only specific
> +			dependencies (subsystems or drivers) that have opted in
> +			will be ignored. This option also dumps out devices
> +			still on the deferred probe list after retrying.

Doesn't sound like a debugging-only option.  I can see devices enabling
this when they figure out that's the only way their platform can boot :)

thanks,

greg k-h

^ permalink raw reply

* uImage target support on arm64
From: Ramon Fried @ 2018-05-24 19:05 UTC (permalink / raw)
  To: linux-arm-kernel

Hi.
I've noticed that it's not supported.
Is it on purpose ?
If not I'll be happy to add support for: make uImage
for arm64 targets.

Warm regards,
Ramon.

^ permalink raw reply

* [PATCH 0/4] arm64: align KPTI interface with x86
From: Mark Langsdorf @ 2018-05-24 19:09 UTC (permalink / raw)
  To: linux-arm-kernel

ARM64 supports KPTI, but is missing some user interface features such as
a debugfs entry compared to the x86 implementation.

Add the nopti argument, update the documentation so that ARM64 as well
as x86 supports nopti and kpti, and add a debugfs entry to check the
status of the kpti on a running machine.

--Mark Langsdorf

^ permalink raw reply

* [PATCH 1/4] arm64: capabilities: add nopti command line argument
From: Mark Langsdorf @ 2018-05-24 19:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524190932.32118-1-mlangsdo@redhat.com>

The x86 kernel and the documentation use 'nopti' as the kernel command
line argument to disable kernel page table isolation, so add nopti to
the arm64 kernel for compatibility.

Signed-off-by: Mark Langsdorf <mlangsdo@redhat.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  6 +++---
 arch/arm64/kernel/cpufeature.c                  | 11 ++++++++++-
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f2040d4..a987725 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3342,8 +3342,8 @@
 	pt.		[PARIDE]
 			See Documentation/blockdev/paride.txt.
 
-	pti=		[X86_64] Control Page Table Isolation of user and
-			kernel address spaces.  Disabling this feature
+	pti=		[X86_64, ARM64] Control Page Table Isolation of user
+			and kernel address spaces.  Disabling this feature
 			removes hardening, but improves performance of
 			system calls and interrupts.
 
@@ -3354,7 +3354,7 @@
 
 			Not specifying this option is equivalent to pti=auto.
 
-	nopti		[X86_64]
+	nopti		[X86_64, ARM64]
 			Equivalent to pti=off
 
 	pty.legacy_count=
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9d1b06d..7c5d8712 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -934,10 +934,19 @@ static int __init parse_kpti(char *str)
 	if (ret)
 		return ret;
 
-	__kpti_forced = enabled ? 1 : -1;
+	if (!__kpti_forced)
+		__kpti_forced = enabled ? 1 : -1;
 	return 0;
 }
 __setup("kpti=", parse_kpti);
+
+/* for compatibility with documentation and x86 nopti command line arg */
+static int __init force_nokpti(char *arg)
+{
+	__kpti_forced = -1;
+	return 0;
+}
+early_param("nopti", force_nokpti);
 #endif	/* CONFIG_UNMAP_KERNEL_AT_EL0 */
 
 #ifdef CONFIG_ARM64_HW_AFDBM
-- 
2.9.5

^ permalink raw reply related

* [PATCH 2/4] arm64: kdebugfs: create arm64 debugfs directory
From: Mark Langsdorf @ 2018-05-24 19:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524190932.32118-1-mlangsdo@redhat.com>

Create debugfs directory for arm64, like the existing one for x86.

Signed-off-by: Mark Langsdorf <mlangsdo@redhat.com>
---
 arch/arm64/kernel/Makefile   |  3 ++-
 arch/arm64/kernel/kdebugfs.c | 27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/kernel/kdebugfs.c

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index bf825f3..a48b298 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -18,7 +18,8 @@ arm64-obj-y		:= debug-monitors.o entry.o irq.o fpsimd.o		\
 			   hyp-stub.o psci.o cpu_ops.o insn.o	\
 			   return_address.o cpuinfo.o cpu_errata.o		\
 			   cpufeature.o alternative.o cacheinfo.o		\
-			   smp.o smp_spin_table.o topology.o smccc-call.o
+			   smp.o smp_spin_table.o topology.o smccc-call.o	\
+			   kdebugfs.o
 
 extra-$(CONFIG_EFI)			:= efi-entry.o
 
diff --git a/arch/arm64/kernel/kdebugfs.c b/arch/arm64/kernel/kdebugfs.c
new file mode 100644
index 0000000..e1e3332
--- /dev/null
+++ b/arch/arm64/kernel/kdebugfs.c
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Architecture specific debugfs files
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ */
+
+#include <linux/debugfs.h>
+#include <linux/export.h>
+#include <linux/init.h>
+
+struct dentry *arch_debugfs_dir;
+EXPORT_SYMBOL(arch_debugfs_dir);
+
+static int __init arch_kdebugfs_init(void)
+{
+	int error = 0;
+
+	arch_debugfs_dir = debugfs_create_dir("arm64", NULL);
+	if (IS_ERR(arch_debugfs_dir)) {
+		arch_debugfs_dir = NULL;
+		return -ENOMEM;
+	}
+
+	return error;
+}
+postcore_initcall(arch_kdebugfs_init);
-- 
2.9.5

^ permalink raw reply related

* [PATCH 3/4] arm64: cpufeature: add debugfs interface for KPTI
From: Mark Langsdorf @ 2018-05-24 19:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524190932.32118-1-mlangsdo@redhat.com>

Add a debugfs interface to check if KPTI is enabled or disabled:
  cat /sys/kernel/debug/arm64/pti_enabled

Slightly rework unmap_kernel_at_el0 logic to simplify calculating if
KPTI is enabled.

Signed-off-by: Mark Langsdorf <mlangsdo@redhat.com>
---
 arch/arm64/kernel/cpufeature.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 7c5d8712..697a6ef 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -24,6 +24,7 @@
 #include <linux/stop_machine.h>
 #include <linux/types.h>
 #include <linux/mm.h>
+#include <linux/debugfs.h>
 #include <asm/cpu.h>
 #include <asm/cpufeature.h>
 #include <asm/cpu_ops.h>
@@ -860,6 +861,7 @@ static bool has_cache_dic(const struct arm64_cpu_capabilities *entry,
 
 #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
 static int __kpti_forced; /* 0: not forced, >0: forced on, <0: forced off */
+static bool __pti_enabled;
 
 static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
 				int scope)
@@ -884,21 +886,21 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
 
 	/* Forced? */
 	if (__kpti_forced) {
+		__pti_enabled = __kpti_forced > 0;
 		pr_info_once("kernel page table isolation forced %s by %s\n",
-			     __kpti_forced > 0 ? "ON" : "OFF", str);
-		return __kpti_forced > 0;
+			     __pti_enabled ? "ON" : "OFF", str);
 	}
-
 	/* Useful for KASLR robustness */
-	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
-		return true;
-
+	else if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
+		__pti_enabled = true;
 	/* Don't force KPTI for CPUs that are not vulnerable */
-	if (is_midr_in_range_list(read_cpuid_id(), kpti_safe_list))
-		return false;
-
+	else if (is_midr_in_range_list(read_cpuid_id(), kpti_safe_list))
+		__pti_enabled = false;
 	/* Defer to CPU feature registers */
-	return !has_cpuid_feature(entry, scope);
+	else
+		__pti_enabled = !has_cpuid_feature(entry, scope);
+
+	return __pti_enabled;
 }
 
 static void
@@ -947,6 +949,14 @@ static int __init force_nokpti(char *arg)
 	return 0;
 }
 early_param("nopti", force_nokpti);
+
+static int __init create_kpti_enabled(void)
+{
+	debugfs_create_bool("pti_enabled", S_IRUSR,
+			   arch_debugfs_dir, &__pti_enabled);
+	return 0;
+}
+late_initcall(create_kpti_enabled);
 #endif	/* CONFIG_UNMAP_KERNEL_AT_EL0 */
 
 #ifdef CONFIG_ARM64_HW_AFDBM
-- 
2.9.5

^ permalink raw reply related

* [PATCH 4/4] arm64: cpufeature: always log KPTI setting on boot
From: Mark Langsdorf @ 2018-05-24 19:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524190932.32118-1-mlangsdo@redhat.com>

Always log KPTI setting at boot time, whether or not KPTI was forced
by a kernel parameter.

Signed-off-by: Mark Langsdorf <mlangsdo@redhat.com>
---
 arch/arm64/kernel/cpufeature.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 697a6ef..e50bf3c 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -889,16 +889,20 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
 		__pti_enabled = __kpti_forced > 0;
 		pr_info_once("kernel page table isolation forced %s by %s\n",
 			     __pti_enabled ? "ON" : "OFF", str);
+	} else {
+		/* Useful for KASLR robustness */
+		if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
+			__pti_enabled = true;
+		/* Don't force KPTI for CPUs that are not vulnerable */
+		else if (is_midr_in_range_list(read_cpuid_id(), kpti_safe_list))
+			__pti_enabled = false;
+		/* Defer to CPU feature registers */
+		else
+			__pti_enabled = !has_cpuid_feature(entry, scope);
+
+		pr_info_once("kernel page table isolation %s by %s\n",
+			    __pti_enabled ? "ON" : "OFF", str);
 	}
-	/* Useful for KASLR robustness */
-	else if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
-		__pti_enabled = true;
-	/* Don't force KPTI for CPUs that are not vulnerable */
-	else if (is_midr_in_range_list(read_cpuid_id(), kpti_safe_list))
-		__pti_enabled = false;
-	/* Defer to CPU feature registers */
-	else
-		__pti_enabled = !has_cpuid_feature(entry, scope);
 
 	return __pti_enabled;
 }
-- 
2.9.5

^ permalink raw reply related

* uImage target support on arm64
From: Baruch Siach @ 2018-05-24 19:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CA+Kvs9=mgW4Z=UcBZZLL82fJ=c--OXF3=B2Zbho+CfMTr8Wk-Q@mail.gmail.com>

Hi Ramon,

On Thu, May 24, 2018 at 10:05:15PM +0300, Ramon Fried wrote:
> I've noticed that it's not supported.
> Is it on purpose ?

Yes. The 32bit load address in the uImage header in pretty limited when 
applied to 64bit ARM64. Even for ARM zImage is the preferred kernel format for 
quite some time now, since it allows flexible load address, as well as 
multi-platform kernels.

> If not I'll be happy to add support for: make uImage
> for arm64 targets.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

^ permalink raw reply

* [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Rob Herring @ 2018-05-24 19:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524185639.GA31019@kroah.com>

On Thu, May 24, 2018 at 1:56 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>> Deferred probe will currently wait forever on dependent devices to probe,
>> but sometimes a driver will never exist. It's also not always critical for
>> a driver to exist. Platforms can rely on default configuration from the
>> bootloader or reset defaults for things such as pinctrl and power domains.
>> This is often the case with initial platform support until various drivers
>> get enabled. There's at least 2 scenarios where deferred probe can render
>> a platform broken. Both involve using a DT which has more devices and
>> dependencies than the kernel supports. The 1st case is a driver may be
>> disabled in the kernel config. The 2nd case is the kernel version may
>> simply not have the dependent driver. This can happen if using a newer DT
>> (provided by firmware perhaps) with a stable kernel version.
>>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>>
>> Cc: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>>  drivers/base/dd.c      | 17 +++++++++++++++++
>>  include/linux/device.h |  2 ++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index c9f54089429b..d6034718da6f 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>       driver_deferred_probe_trigger();
>>  }
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
>> +{
>> +     if (optional && initcalls_done) {
>> +             dev_WARN(dev, "ignoring dependency for device, assuming no driver");
>
> You really only need dev_warn(), here, right?

No, the screaming is on purpose.

Bjorn had concerns about debugging/supporting subtle problems that
could stem from this. Such as electrical settings not quite right that
cause intermittent errors.

Rob

^ permalink raw reply

* [PATCH v2 2/8] driver core: add a deferred probe timeout
From: Rob Herring @ 2018-05-24 19:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524190126.GC31019@kroah.com>

On Thu, May 24, 2018 at 2:01 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, May 24, 2018 at 12:50:18PM -0500, Rob Herring wrote:
>> Deferring probe can wait forever on dependencies that may never appear
>> for a variety of reasons. This can be difficult to debug especially if
>> the console has dependencies or userspace fails to boot to a shell. Add
>> a timeout to retry probing without possibly optional dependencies and to
>> dump out the deferred probe pending list after retrying.
>>
>> This mechanism is intended for debug purposes. It won't work for the
>> console which needs to be enabled before userspace starts. However, if
>> the console's dependencies are resolved, then the kernel log will be
>> printed (as opposed to no output).
>>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>>  .../admin-guide/kernel-parameters.txt         |  7 +++++
>>  drivers/base/dd.c                             | 28 ++++++++++++++++++-
>>  2 files changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 11fc28ecdb6d..dd3f40b34a24 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -809,6 +809,13 @@
>>                       Defaults to the default architecture's huge page size
>>                       if not specified.
>>
>> +     deferred_probe_timeout=
>> +                     [KNL] Set a timeout in seconds for deferred probe to
>> +                     give up waiting on dependencies to probe. Only specific
>> +                     dependencies (subsystems or drivers) that have opted in
>> +                     will be ignored. This option also dumps out devices
>> +                     still on the deferred probe list after retrying.
>
> Doesn't sound like a debugging-only option.  I can see devices enabling
> this when they figure out that's the only way their platform can boot :)

Here's some rope...

No doubt it can be abused. So are you saying don't call it a debug
option or hide it behind a config option? And for the latter, what's
one that distros don't just turn on?

Rob

^ permalink raw reply

* [RESEND PATCH] ARM: dts: imx6q: Use correct SDMA script for SPI5 core
From: Sean Nyekjaer @ 2018-05-24 19:46 UTC (permalink / raw)
  To: linux-arm-kernel

According to the reference manual the shp_2_mcu / mcu_2_shp
scripts must be used for devices connected through the SPBA.

This fixes an issue we saw with DMA transfers.
Sometimes the SPI controller RX FIFO was not empty after a DMA
transfer and the driver got stuck in the next PIO transfer when
it read one word more than expected.

commit dd4b487b32a35 ("ARM: dts: imx6: Use correct SDMA script
for SPI cores") is fixing the same issue but only for SPI1 - 4.

Fixes: 677940258dd8e ("ARM: dts: imx6q: enable dma for ecspi5")

Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/boot/dts/imx6q.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index ae7b3f107893..5185300cc11f 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -96,7 +96,7 @@
 					clocks = <&clks IMX6Q_CLK_ECSPI5>,
 						 <&clks IMX6Q_CLK_ECSPI5>;
 					clock-names = "ipg", "per";
-					dmas = <&sdma 11 7 1>, <&sdma 12 7 2>;
+					dmas = <&sdma 11 8 1>, <&sdma 12 8 2>;
 					dma-names = "rx", "tx";
 					status = "disabled";
 				};
-- 
2.17.0

^ permalink raw reply related

* [PATCH v2 2/8] driver core: add a deferred probe timeout
From: Greg Kroah-Hartman @ 2018-05-24 19:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAL_Jsq+wMJ+9JTGFGSsSW2Vww_mvAkzc3ujmftgbpQxDTrLHag@mail.gmail.com>

On Thu, May 24, 2018 at 02:45:48PM -0500, Rob Herring wrote:
> On Thu, May 24, 2018 at 2:01 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Thu, May 24, 2018 at 12:50:18PM -0500, Rob Herring wrote:
> >> Deferring probe can wait forever on dependencies that may never appear
> >> for a variety of reasons. This can be difficult to debug especially if
> >> the console has dependencies or userspace fails to boot to a shell. Add
> >> a timeout to retry probing without possibly optional dependencies and to
> >> dump out the deferred probe pending list after retrying.
> >>
> >> This mechanism is intended for debug purposes. It won't work for the
> >> console which needs to be enabled before userspace starts. However, if
> >> the console's dependencies are resolved, then the kernel log will be
> >> printed (as opposed to no output).
> >>
> >> Signed-off-by: Rob Herring <robh@kernel.org>
> >> ---
> >>  .../admin-guide/kernel-parameters.txt         |  7 +++++
> >>  drivers/base/dd.c                             | 28 ++++++++++++++++++-
> >>  2 files changed, 34 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> >> index 11fc28ecdb6d..dd3f40b34a24 100644
> >> --- a/Documentation/admin-guide/kernel-parameters.txt
> >> +++ b/Documentation/admin-guide/kernel-parameters.txt
> >> @@ -809,6 +809,13 @@
> >>                       Defaults to the default architecture's huge page size
> >>                       if not specified.
> >>
> >> +     deferred_probe_timeout=
> >> +                     [KNL] Set a timeout in seconds for deferred probe to
> >> +                     give up waiting on dependencies to probe. Only specific
> >> +                     dependencies (subsystems or drivers) that have opted in
> >> +                     will be ignored. This option also dumps out devices
> >> +                     still on the deferred probe list after retrying.
> >
> > Doesn't sound like a debugging-only option.  I can see devices enabling
> > this when they figure out that's the only way their platform can boot :)
> 
> Here's some rope...
> 
> No doubt it can be abused. So are you saying don't call it a debug
> option or hide it behind a config option? And for the latter, what's
> one that distros don't just turn on?

If it is a debug option, make it obvious it's only for debugging.  The
commit log here says it's a debug option, but your documentation does
not say that at all, and that's what people will read.

Well, they might read it, probably not.  But at least give us something
to point at when they mess things up :)

thanks,

greg k-h

^ permalink raw reply

* [PATCH 10/11] coresight: tmc-etr: Add transparent buffer management
From: Mathieu Poirier @ 2018-05-24 19:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526661567-4578-11-git-send-email-suzuki.poulose@arm.com>

On Fri, May 18, 2018 at 05:39:26PM +0100, Suzuki K Poulose wrote:
> At the moment we always use contiguous memory for TMC ETR tracing
> when used from sysfs. The size of the buffer is fixed at boot time
> and can only be changed by modifiying the DT. With the introduction
> of SG support we could support really large buffers in that mode.
> This patch abstracts the buffer used for ETR to switch between a
> contiguous buffer or a SG table depending on the availability of
> the memory.
> 
> This also enables the sysfs mode to use the ETR in SG mode depending
> on configured the trace buffer size. Also, since ETR will use the
> new infrastructure to manage the buffer, we can get rid of some
> of the members in the tmc_drvdata and clean up the fields a bit.

Upon first reading this changelog I thought this patch does way too many things
but after looking at the content it isn't the case.  We could try to split the
patch by moving the introduction of the SG operations to another patch but it
would save about 60 lines, which is hardly worth it.  

As it stands now it is alsmost guaranted other reviewers will ask you to split
your work.  Perhaps rephrasing the changelog to concentrate on the global idea
of what the patch does will help (just my personnal opinion).  

> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-tmc-etr.c | 450 +++++++++++++++++++-----
>  drivers/hwtracing/coresight/coresight-tmc.h     |  57 ++-
>  2 files changed, 418 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 7ab0fd1..143afba 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -17,10 +17,18 @@
>  
>  #include <linux/coresight.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/iommu.h>
>  #include <linux/slab.h>
>  #include "coresight-priv.h"
>  #include "coresight-tmc.h"
>  
> +struct etr_flat_buf {
> +	struct device	*dev;
> +	dma_addr_t	daddr;
> +	void		*vaddr;
> +	size_t		size;
> +};
> +
>  /*
>   * The TMC ETR SG has a page size of 4K. The SG table contains pointers
>   * to 4KB buffers. However, the OS may use a PAGE_SIZE different from
> @@ -541,7 +549,7 @@ static void tmc_etr_sg_table_populate(struct etr_sg_table *etr_table)
>   * @size	- Total size of the data buffer
>   * @pages	- Optional list of page virtual address
>   */
> -static struct etr_sg_table __maybe_unused *
> +static struct etr_sg_table *
>  tmc_init_etr_sg_table(struct device *dev, int node,
>  		  unsigned long size, void **pages)
>  {
> @@ -573,16 +581,307 @@ tmc_init_etr_sg_table(struct device *dev, int node,
>  	return etr_table;
>  }
>  
> +/*
> + * tmc_etr_alloc_flat_buf: Allocate a contiguous DMA buffer.
> + */
> +static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata,
> +				  struct etr_buf *etr_buf, int node,
> +				  void **pages)
> +{
> +	struct etr_flat_buf *flat_buf;
> +
> +	/* We cannot reuse existing pages for flat buf */
> +	if (pages)
> +		return -EINVAL;

Perfect.

> +
> +	flat_buf = kzalloc(sizeof(*flat_buf), GFP_KERNEL);
> +	if (!flat_buf)
> +		return -ENOMEM;
> +
> +	flat_buf->vaddr = dma_alloc_coherent(drvdata->dev, etr_buf->size,
> +					   &flat_buf->daddr, GFP_KERNEL);

Indendation of the second line.

> +	if (!flat_buf->vaddr) {
> +		kfree(flat_buf);
> +		return -ENOMEM;
> +	}
> +
> +	flat_buf->size = etr_buf->size;
> +	flat_buf->dev = drvdata->dev;
> +	etr_buf->hwaddr = flat_buf->daddr;
> +	etr_buf->mode = ETR_MODE_FLAT;
> +	etr_buf->private = flat_buf;
> +	return 0;
> +}
> +
> +static void tmc_etr_free_flat_buf(struct etr_buf *etr_buf)
> +{
> +	struct etr_flat_buf *flat_buf = etr_buf->private;
> +
> +	if (flat_buf && flat_buf->daddr)
> +		dma_free_coherent(flat_buf->dev, flat_buf->size,
> +				  flat_buf->vaddr, flat_buf->daddr);
> +	kfree(flat_buf);
> +}
> +
> +static void tmc_etr_sync_flat_buf(struct etr_buf *etr_buf, u64 rrp, u64 rwp)
> +{
> +	/*
> +	 * Adjust the buffer to point to the beginning of the trace data
> +	 * and update the available trace data.
> +	 */
> +	etr_buf->offset = rrp - etr_buf->hwaddr;
> +	if (etr_buf->full)
> +		etr_buf->len = etr_buf->size;
> +	else
> +		etr_buf->len = rwp - rrp;
> +}
> +
> +static ssize_t tmc_etr_get_data_flat_buf(struct etr_buf *etr_buf,
> +					 u64 offset, size_t len, char **bufpp)
> +{
> +	struct etr_flat_buf *flat_buf = etr_buf->private;
> +
> +	*bufpp = (char *)flat_buf->vaddr + offset;
> +	/*
> +	 * tmc_etr_buf_get_data already adjusts the length to handle
> +	 * buffer wrapping around.
> +	 */
> +	return len;
> +}
> +
> +static const struct etr_buf_operations etr_flat_buf_ops = {
> +	.alloc = tmc_etr_alloc_flat_buf,
> +	.free = tmc_etr_free_flat_buf,
> +	.sync = tmc_etr_sync_flat_buf,
> +	.get_data = tmc_etr_get_data_flat_buf,
> +};
> +
> +/*
> + * tmc_etr_alloc_sg_buf: Allocate an SG buf @etr_buf. Setup the parameters
> + * appropriately.
> + */
> +static int tmc_etr_alloc_sg_buf(struct tmc_drvdata *drvdata,
> +				struct etr_buf *etr_buf, int node,
> +				void **pages)
> +{
> +	struct etr_sg_table *etr_table;
> +
> +	etr_table = tmc_init_etr_sg_table(drvdata->dev, node,
> +					  etr_buf->size, pages);
> +	if (IS_ERR(etr_table))
> +		return -ENOMEM;
> +	etr_buf->hwaddr = etr_table->hwaddr;
> +	etr_buf->mode = ETR_MODE_ETR_SG;
> +	etr_buf->private = etr_table;
> +	return 0;
> +}
> +
> +static void tmc_etr_free_sg_buf(struct etr_buf *etr_buf)
> +{
> +	struct etr_sg_table *etr_table = etr_buf->private;
> +
> +	if (etr_table) {
> +		tmc_free_sg_table(etr_table->sg_table);
> +		kfree(etr_table);
> +	}
> +}
> +
> +static ssize_t tmc_etr_get_data_sg_buf(struct etr_buf *etr_buf, u64 offset,
> +				       size_t len, char **bufpp)
> +{
> +	struct etr_sg_table *etr_table = etr_buf->private;
> +
> +	return tmc_sg_table_get_data(etr_table->sg_table, offset, len, bufpp);
> +}
> +
> +static void tmc_etr_sync_sg_buf(struct etr_buf *etr_buf, u64 rrp, u64 rwp)
> +{
> +	long r_offset, w_offset;
> +	struct etr_sg_table *etr_table = etr_buf->private;
> +	struct tmc_sg_table *table = etr_table->sg_table;
> +
> +	/* Convert hw address to offset in the buffer */
> +	r_offset = tmc_sg_get_data_page_offset(table, rrp);
> +	if (r_offset < 0) {
> +		dev_warn(table->dev,
> +			 "Unable to map RRP %llx to offset\n", rrp);
> +		etr_buf->len = 0;
> +		return;
> +	}
> +
> +	w_offset = tmc_sg_get_data_page_offset(table, rwp);
> +	if (w_offset < 0) {
> +		dev_warn(table->dev,
> +			 "Unable to map RWP %llx to offset\n", rwp);
> +		etr_buf->len = 0;
> +		return;
> +	}
> +
> +	etr_buf->offset = r_offset;
> +	if (etr_buf->full)
> +		etr_buf->len = etr_buf->size;
> +	else
> +		etr_buf->len = ((w_offset < r_offset) ? etr_buf->size : 0) +
> +				w_offset - r_offset;
> +	tmc_sg_table_sync_data_range(table, r_offset, etr_buf->len);
> +}
> +
> +static const struct etr_buf_operations etr_sg_buf_ops = {
> +	.alloc = tmc_etr_alloc_sg_buf,
> +	.free = tmc_etr_free_sg_buf,
> +	.sync = tmc_etr_sync_sg_buf,
> +	.get_data = tmc_etr_get_data_sg_buf,
> +};
> +
> +static const struct etr_buf_operations *etr_buf_ops[] = {
> +	[ETR_MODE_FLAT] = &etr_flat_buf_ops,
> +	[ETR_MODE_ETR_SG] = &etr_sg_buf_ops,
> +};
> +
> +static inline int tmc_etr_mode_alloc_buf(int mode,
> +					 struct tmc_drvdata *drvdata,
> +					 struct etr_buf *etr_buf, int node,
> +					 void **pages)
> +{
> +	int rc;
> +
> +	switch (mode) {
> +	case ETR_MODE_FLAT:
> +	case ETR_MODE_ETR_SG:
> +		rc = etr_buf_ops[mode]->alloc(drvdata, etr_buf, node, pages);
> +		if (!rc)
> +			etr_buf->ops = etr_buf_ops[mode];
> +		return rc;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +/*
> + * tmc_alloc_etr_buf: Allocate a buffer use by ETR.
> + * @drvdata	: ETR device details.
> + * @size	: size of the requested buffer.
> + * @flags	: Required properties for the buffer.
> + * @node	: Node for memory allocations.
> + * @pages	: An optional list of pages.
> + */
> +static struct etr_buf *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata,
> +					 ssize_t size, int flags,
> +					 int node, void **pages)
> +{
> +	int rc = -ENOMEM;
> +	bool has_etr_sg, has_iommu;
> +	struct etr_buf *etr_buf;
> +
> +	has_etr_sg = tmc_etr_has_cap(drvdata, TMC_ETR_SG);
> +	has_iommu = iommu_get_domain_for_dev(drvdata->dev);
> +
> +	etr_buf = kzalloc(sizeof(*etr_buf), GFP_KERNEL);
> +	if (!etr_buf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	etr_buf->size = size;
> +
> +	/*
> +	 * If we have to use an existing list of pages, we cannot reliably
> +	 * use a contiguous DMA memory (even if we have an IOMMU). Otherwise,
> +	 * we use the contiguous DMA memory if at least one of the following
> +	 * conditions is true:
> +	 *  a) The ETR cannot use Scatter-Gather.
> +	 *  b) we have a backing IOMMU
> +	 *  c) The requested memory size is smaller (< 1M).
> +	 *
> +	 * Fallback to available mechanisms.
> +	 *
> +	 */
> +	if (!pages &&
> +	    (!has_etr_sg || has_iommu || size < SZ_1M))
> +		rc = tmc_etr_mode_alloc_buf(ETR_MODE_FLAT, drvdata,
> +					    etr_buf, node, pages);
> +	if (rc && has_etr_sg)
> +		rc = tmc_etr_mode_alloc_buf(ETR_MODE_ETR_SG, drvdata,
> +					    etr_buf, node, pages);
> +	if (rc) {
> +		kfree(etr_buf);
> +		return ERR_PTR(rc);
> +	}
> +
> +	return etr_buf;
> +}
> +
> +static void tmc_free_etr_buf(struct etr_buf *etr_buf)
> +{
> +	WARN_ON(!etr_buf->ops || !etr_buf->ops->free);
> +	etr_buf->ops->free(etr_buf);
> +	kfree(etr_buf);
> +}
> +
> +/*
> + * tmc_etr_buf_get_data: Get the pointer the trace data at @offset
> + * with a maximum of @len bytes.
> + * Returns: The size of the linear data available @pos, with *bufpp
> + * updated to point to the buffer.
> + */
> +static ssize_t tmc_etr_buf_get_data(struct etr_buf *etr_buf,
> +				    u64 offset, size_t len, char **bufpp)
> +{
> +	/* Adjust the length to limit this transaction to end of buffer */
> +	len = (len < (etr_buf->size - offset)) ? len : etr_buf->size - offset;
> +
> +	return etr_buf->ops->get_data(etr_buf, (u64)offset, len, bufpp);
> +}
> +
> +static inline s64
> +tmc_etr_buf_insert_barrier_packet(struct etr_buf *etr_buf, u64 offset)
> +{
> +	ssize_t len;
> +	char *bufp;
> +
> +	len = tmc_etr_buf_get_data(etr_buf, offset,
> +				   CORESIGHT_BARRIER_PKT_SIZE, &bufp);
> +	if (WARN_ON(len <= CORESIGHT_BARRIER_PKT_SIZE))
> +		return -EINVAL;
> +	coresight_insert_barrier_packet(bufp);
> +	return offset + CORESIGHT_BARRIER_PKT_SIZE;
> +}
> +
> +/*
> + * tmc_sync_etr_buf: Sync the trace buffer availability with drvdata.
> + * Makes sure the trace data is synced to the memory for consumption.
> + * @etr_buf->offset will hold the offset to the beginning of the trace data
> + * within the buffer, with @etr_buf->len bytes to consume.
> + */
> +static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata)
> +{
> +	struct etr_buf *etr_buf = drvdata->etr_buf;
> +	u64 rrp, rwp;
> +	u32 status;
> +
> +	rrp = tmc_read_rrp(drvdata);
> +	rwp = tmc_read_rwp(drvdata);
> +	status = readl_relaxed(drvdata->base + TMC_STS);
> +	etr_buf->full = status & TMC_STS_FULL;
> +
> +	WARN_ON(!etr_buf->ops || !etr_buf->ops->sync);
> +
> +	etr_buf->ops->sync(etr_buf, rrp, rwp);
> +
> +	/* Insert barrier packets at the beginning, if there was an overflow */
> +	if (etr_buf->full)
> +		tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset);
> +}
> +
>  static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
>  {
>  	u32 axictl, sts;
> +	struct etr_buf *etr_buf = drvdata->etr_buf;
>  
>  	CS_UNLOCK(drvdata->base);
>  
>  	/* Wait for TMCSReady bit to be set */
>  	tmc_wait_for_tmcready(drvdata);
>  
> -	writel_relaxed(drvdata->size / 4, drvdata->base + TMC_RSZ);
> +	writel_relaxed(etr_buf->size / 4, drvdata->base + TMC_RSZ);
>  	writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base + TMC_MODE);
>  
>  	axictl = readl_relaxed(drvdata->base + TMC_AXICTL);
> @@ -595,16 +894,22 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
>  		axictl |= TMC_AXICTL_ARCACHE_OS;
>  	}
>  
> +	if (etr_buf->mode == ETR_MODE_ETR_SG) {
> +		if (WARN_ON(!tmc_etr_has_cap(drvdata, TMC_ETR_SG)))
> +			return;
> +		axictl |= TMC_AXICTL_SCT_GAT_MODE;
> +	}
> +
>  	writel_relaxed(axictl, drvdata->base + TMC_AXICTL);
> -	tmc_write_dba(drvdata, drvdata->paddr);
> +	tmc_write_dba(drvdata, etr_buf->hwaddr);
>  	/*
>  	 * If the TMC pointers must be programmed before the session,
>  	 * we have to set it properly (i.e, RRP/RWP to base address and
>  	 * STS to "not full").
>  	 */
>  	if (tmc_etr_has_cap(drvdata, TMC_ETR_SAVE_RESTORE)) {
> -		tmc_write_rrp(drvdata, drvdata->paddr);
> -		tmc_write_rwp(drvdata, drvdata->paddr);
> +		tmc_write_rrp(drvdata, etr_buf->hwaddr);
> +		tmc_write_rwp(drvdata, etr_buf->hwaddr);
>  		sts = readl_relaxed(drvdata->base + TMC_STS) & ~TMC_STS_FULL;
>  		writel_relaxed(sts, drvdata->base + TMC_STS);
>  	}
> @@ -620,63 +925,53 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
>  }
>  
>  /*
> - * Return the available trace data in the buffer @pos, with a maximum
> - * limit of @len, also updating the @bufpp on where to find it.
> + * Return the available trace data in the buffer (starts at etr_buf->offset,
> + * limited by etr_buf->len) from @pos, with a maximum limit of @len,
> + * also updating the @bufpp on where to find it. Since the trace data
> + * starts at anywhere in the buffer, depending on the RRP, we adjust the
> + * @len returned to handle buffer wrapping around.
>   */
>  ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
>  				loff_t pos, size_t len, char **bufpp)
>  {
> +	s64 offset;
>  	ssize_t actual = len;
> -	char *bufp = drvdata->buf + pos;
> -	char *bufend = (char *)(drvdata->vaddr + drvdata->size);
> -
> -	/* Adjust the len to available size @pos */
> -	if (pos + actual > drvdata->len)
> -		actual = drvdata->len - pos;
> +	struct etr_buf *etr_buf = drvdata->etr_buf;
>  
> +	if (pos + actual > etr_buf->len)
> +		actual = etr_buf->len - pos;
>  	if (actual <= 0)
>  		return actual;
>  
> -	/*
> -	 * Since we use a circular buffer, with trace data starting
> -	 * @drvdata->buf, possibly anywhere in the buffer @drvdata->vaddr,
> -	 * wrap the current @pos to within the buffer.
> -	 */
> -	if (bufp >= bufend)
> -		bufp -= drvdata->size;
> -	/*
> -	 * For simplicity, avoid copying over a wrapped around buffer.
> -	 */
> -	if ((bufp + actual) > bufend)
> -		actual = bufend - bufp;
> -	*bufpp = bufp;
> -	return actual;
> +	/* Compute the offset from which we read the data */
> +	offset = etr_buf->offset + pos;
> +	if (offset >= etr_buf->size)
> +		offset -= etr_buf->size;
> +	return tmc_etr_buf_get_data(etr_buf, offset, actual, bufpp);
>  }
>  
> -static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata)
> +static struct etr_buf *
> +tmc_etr_setup_sysfs_buf(struct tmc_drvdata *drvdata)
>  {
> -	u32 val;
> -	u64 rwp;
> +	return tmc_alloc_etr_buf(drvdata, drvdata->size,
> +				 0, cpu_to_node(0), NULL);
> +}
>  
> -	rwp = tmc_read_rwp(drvdata);
> -	val = readl_relaxed(drvdata->base + TMC_STS);
> +static void
> +tmc_etr_free_sysfs_buf(struct etr_buf *buf)
> +{
> +	if (buf)
> +		tmc_free_etr_buf(buf);
> +}
>  
> -	/*
> -	 * Adjust the buffer to point to the beginning of the trace data
> -	 * and update the available trace data.
> -	 */
> -	if (val & TMC_STS_FULL) {
> -		drvdata->buf = drvdata->vaddr + rwp - drvdata->paddr;
> -		drvdata->len = drvdata->size;
> -		coresight_insert_barrier_packet(drvdata->buf);
> -	} else {
> -		drvdata->buf = drvdata->vaddr;
> -		drvdata->len = rwp - drvdata->paddr;
> -	}
> +static void tmc_etr_sync_sysfs_buf(struct tmc_drvdata *drvdata)
> +{
> +	tmc_sync_etr_buf(drvdata);
>  }
>  
>  static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
>  {
> +
>  	CS_UNLOCK(drvdata->base);
>  
>  	tmc_flush_and_stop(drvdata);
> @@ -685,7 +980,8 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
>  	 * read before the TMC is disabled.
>  	 */
>  	if (drvdata->mode == CS_MODE_SYSFS)
> -		tmc_etr_dump_hw(drvdata);
> +		tmc_etr_sync_sysfs_buf(drvdata);
> +
>  	tmc_disable_hw(drvdata);
>  
>  	CS_LOCK(drvdata->base);
> @@ -696,34 +992,31 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
>  	int ret = 0;
>  	bool used = false;
>  	unsigned long flags;
> -	void __iomem *vaddr = NULL;
> -	dma_addr_t paddr;
>  	struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct etr_buf *new_buf = NULL, *free_buf = NULL;
>  
>  
>  	/*
> -	 * If we don't have a buffer release the lock and allocate memory.
> -	 * Otherwise keep the lock and move along.
> +	 * If we are enabling the ETR from disabled state, we need to make
> +	 * sure we have a buffer with the right size. The etr_buf is not reset
> +	 * immediately after we stop the tracing in SYSFS mode as we wait for
> +	 * the user to collect the data. We may be able to reuse the existing
> +	 * buffer, provided the size matches. Any allocation has to be done
> +	 * with the lock released.
>  	 */
>  	spin_lock_irqsave(&drvdata->spinlock, flags);
> -	if (!drvdata->vaddr) {
> +	if (!drvdata->etr_buf || (drvdata->etr_buf->size != drvdata->size)) {
>  		spin_unlock_irqrestore(&drvdata->spinlock, flags);
> -
> -		/*
> -		 * Contiguous  memory can't be allocated while a spinlock is
> -		 * held.  As such allocate memory here and free it if a buffer
> -		 * has already been allocated (from a previous session).
> -		 */
> -		vaddr = dma_alloc_coherent(drvdata->dev, drvdata->size,
> -					   &paddr, GFP_KERNEL);
> -		if (!vaddr)
> -			return -ENOMEM;
> +		/* Allocate memory with the spinlock released */
> +		free_buf = new_buf = tmc_etr_setup_sysfs_buf(drvdata);
> +		if (IS_ERR(new_buf))
> +			return PTR_ERR(new_buf);
>  
>  		/* Let's try again */
>  		spin_lock_irqsave(&drvdata->spinlock, flags);
>  	}
>  
> -	if (drvdata->reading) {
> +	if (drvdata->reading || drvdata->mode == CS_MODE_PERF) {
>  		ret = -EBUSY;
>  		goto out;
>  	}
> @@ -731,21 +1024,20 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
>  	/*
>  	 * In sysFS mode we can have multiple writers per sink.  Since this
>  	 * sink is already enabled no memory is needed and the HW need not be
> -	 * touched.
> +	 * touched, even if the buffer size has changed.
>  	 */
>  	if (drvdata->mode == CS_MODE_SYSFS)
>  		goto out;
>  
>  	/*
> -	 * If drvdata::buf == NULL, use the memory allocated above.
> -	 * Otherwise a buffer still exists from a previous session, so
> -	 * simply use that.
> +	 * If we don't have a buffer or it doesn't match the requested size,
> +	 * use the memory allocated above. Otherwise reuse it.
>  	 */
> -	if (drvdata->buf == NULL) {
> +	if (!drvdata->etr_buf ||
> +	    (new_buf && drvdata->etr_buf->size != new_buf->size)) {
>  		used = true;

With the introduction of variable free_buf, this is no longer needed.

> -		drvdata->vaddr = vaddr;
> -		drvdata->paddr = paddr;
> -		drvdata->buf = drvdata->vaddr;
> +		free_buf = drvdata->etr_buf;
> +		drvdata->etr_buf = new_buf;
>  	}
>  
>  	drvdata->mode = CS_MODE_SYSFS;
> @@ -754,8 +1046,8 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
>  	spin_unlock_irqrestore(&drvdata->spinlock, flags);
>  
>  	/* Free memory outside the spinlock if need be */
> -	if (!used && vaddr)
> -		dma_free_coherent(drvdata->dev, drvdata->size, vaddr, paddr);
> +	if (free_buf)
> +		tmc_etr_free_sysfs_buf(free_buf);
>  
>  	if (!ret)
>  		dev_info(drvdata->dev, "TMC-ETR enabled\n");
> @@ -834,8 +1126,8 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
>  		goto out;
>  	}
>  
> -	/* If drvdata::buf is NULL the trace data has been read already */
> -	if (drvdata->buf == NULL) {
> +	/* If drvdata::etr_buf is NULL the trace data has been read already */
> +	if (drvdata->etr_buf == NULL) {

As I pointed out during my last review this hunk doesn't apply on my next
branch and as such can't review this part.

>  		ret = -EINVAL;
>  		goto out;
>  	}
> @@ -854,8 +1146,7 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
>  int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
>  {
>  	unsigned long flags;
> -	dma_addr_t paddr;
> -	void __iomem *vaddr = NULL;
> +	struct etr_buf *etr_buf = NULL;
>  
>  	/* config types are set a boot time and never change */
>  	if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR))
> @@ -876,17 +1167,16 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
>  		 * The ETR is not tracing and the buffer was just read.
>  		 * As such prepare to free the trace buffer.
>  		 */
> -		vaddr = drvdata->vaddr;
> -		paddr = drvdata->paddr;
> -		drvdata->buf = drvdata->vaddr = NULL;
> +		etr_buf =  drvdata->etr_buf;
> +		drvdata->etr_buf = NULL;
>  	}
>  
>  	drvdata->reading = false;
>  	spin_unlock_irqrestore(&drvdata->spinlock, flags);
>  
>  	/* Free allocated memory out side of the spinlock */
> -	if (vaddr)
> -		dma_free_coherent(drvdata->dev, drvdata->size, vaddr, paddr);
> +	if (etr_buf)
> +		tmc_free_etr_buf(etr_buf);
>  
>  	return 0;
>  }
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 19a765c..c00643c 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -55,6 +55,7 @@
>  #define TMC_STS_TMCREADY_BIT	2
>  #define TMC_STS_FULL		BIT(0)
>  #define TMC_STS_TRIGGERED	BIT(1)
> +
>  /*
>   * TMC_AXICTL - 0x110
>   *
> @@ -134,6 +135,35 @@ enum tmc_mem_intf_width {
>  #define CORESIGHT_SOC_600_ETR_CAPS	\
>  	(TMC_ETR_SAVE_RESTORE | TMC_ETR_AXI_ARCACHE)
>  
> +enum etr_mode {
> +	ETR_MODE_FLAT,		/* Uses contiguous flat buffer */
> +	ETR_MODE_ETR_SG,	/* Uses in-built TMC ETR SG mechanism */
> +};
> +
> +struct etr_buf_operations;
> +
> +/**
> + * struct etr_buf - Details of the buffer used by ETR
> + * @mode	: Mode of the ETR buffer, contiguous, Scatter Gather etc.
> + * @full	: Trace data overflow
> + * @size	: Size of the buffer.
> + * @hwaddr	: Address to be programmed in the TMC:DBA{LO,HI}
> + * @offset	: Offset of the trace data in the buffer for consumption.
> + * @len		: Available trace data @buf (may round up to the beginning).
> + * @ops		: ETR buffer operations for the mode.
> + * @private	: Backend specific information for the buf
> + */
> +struct etr_buf {
> +	enum etr_mode			mode;
> +	bool				full;
> +	ssize_t				size;
> +	dma_addr_t			hwaddr;
> +	unsigned long			offset;
> +	s64				len;
> +	const struct etr_buf_operations	*ops;
> +	void				*private;
> +};
> +
>  /**
>   * struct tmc_drvdata - specifics associated to an TMC component
>   * @base:	memory mapped base address for this component.
> @@ -141,11 +171,10 @@ enum tmc_mem_intf_width {
>   * @csdev:	component vitals needed by the framework.
>   * @miscdev:	specifics to handle "/dev/xyz.tmc" entry.
>   * @spinlock:	only one at a time pls.
> - * @buf:	area of memory where trace data get sent.
> - * @paddr:	DMA start location in RAM.
> - * @vaddr:	virtual representation of @paddr.
> - * @size:	trace buffer size.
> - * @len:	size of the available trace.
> + * @buf:	Snapshot of the trace data for ETF/ETB.
> + * @etr_buf:	details of buffer used in TMC-ETR
> + * @len:	size of the available trace for ETF/ETB.
> + * @size:	trace buffer size for this TMC (common for all modes).
>   * @mode:	how this TMC is being used.
>   * @config_type: TMC variant, must be of type @tmc_config_type.
>   * @memwidth:	width of the memory interface databus, in bytes.
> @@ -160,11 +189,12 @@ struct tmc_drvdata {
>  	struct miscdevice	miscdev;
>  	spinlock_t		spinlock;
>  	bool			reading;
> -	char			*buf;
> -	dma_addr_t		paddr;
> -	void __iomem		*vaddr;
> -	u32			size;
> +	union {
> +		char		*buf;		/* TMC ETB */
> +		struct etr_buf	*etr_buf;	/* TMC ETR */
> +	};
>  	u32			len;
> +	u32			size;
>  	u32			mode;
>  	enum tmc_config_type	config_type;
>  	enum tmc_mem_intf_width	memwidth;
> @@ -172,6 +202,15 @@ struct tmc_drvdata {
>  	u32			etr_caps;
>  };
>  
> +struct etr_buf_operations {
> +	int (*alloc)(struct tmc_drvdata *drvdata, struct etr_buf *etr_buf,
> +			int node, void **pages);
> +	void (*sync)(struct etr_buf *etr_buf, u64 rrp, u64 rwp);
> +	ssize_t (*get_data)(struct etr_buf *etr_buf, u64 offset, size_t len,
> +				char **bufpp);
> +	void (*free)(struct etr_buf *etr_buf);
> +};
> +
>  /**
>   * struct tmc_pages - Collection of pages used for SG.
>   * @nr_pages:		Number of pages in the list.
> -- 
> 2.7.4
> 

^ permalink raw reply

* [PATCH v3 2/3] arm64: dts: renesas: draak: Describe CVBS input
From: jacopo mondi @ 2018-05-24 19:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1728709.thk7gXDxlo@avalon>

HI Laurent,

On Mon, May 21, 2018 at 04:10:34PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Monday, 21 May 2018 15:33:40 EEST jacopo mondi wrote:
> > On Mon, May 21, 2018 at 01:54:55PM +0300, Laurent Pinchart wrote:
> > > On Monday, 21 May 2018 12:57:05 EEST jacopo mondi wrote:
> > >> On Fri, May 18, 2018 at 06:12:15PM +0300, Laurent Pinchart wrote:
> > >>> On Friday, 18 May 2018 17:47:57 EEST Jacopo Mondi wrote:
> > >>>> Describe CVBS video input through analog video decoder ADV7180
> > >>>> connected to video input interface VIN4.
> > >>>>
> > >>>> The video input signal path is shared with HDMI video input, and
> > >>>> selected by on-board switches SW-53 and SW-54 with CVBS input
> > >>>> selected by the default switches configuration.
> > >>>>
> > >>>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > >>>> Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
> > >>>>
> > >>>> ---
> > >>>> v2 -> v3:
> > >>>> - Add comment to describe the shared input video path
> > >>>> - Add my SoB and Niklas' R-b tags
> > >>>> ---
> > >>>>
> > >>>>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 42 ++++++++++++++++++
> > >>>>  1 file changed, 42 insertions(+)
> > >>>>
> > >>>> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > >>>> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index
> > >>>> 9d73de8..95745fc
> > >>>> 100644
> > >>>> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > >>>> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > >>>> @@ -142,6 +142,11 @@
> > >>>>  		groups = "usb0";
> > >>>>  		function = "usb0";
> > >>>>  	};
> > >>>> +
> > >>>> +	vin4_pins_cvbs: vin4 {
> > >>>> +		groups = "vin4_data8", "vin4_sync", "vin4_clk";
> > >>>> +		function = "vin4";
> > >>>> +	};
> > >>>>  };
> > >>>>
> > >>>>  &i2c0 {
> > >>>> @@ -154,6 +159,23 @@
> > >>>>  		reg = <0x50>;
> > >>>>  		pagesize = <8>;
> > >>>>  	};
> > >>>> +
> > >>>> +	analog-video at 20 {
> > >>>> +		compatible = "adi,adv7180";
> > >>>> +		reg = <0x20>;
> > >>>> +
> > >>>> +		port {
> > >>>
> > >>> The adv7180 DT bindings document the output port as 3 or 6
> > >>> (respectively for the CP and ST versions of the chip). You should thus
> > >>> number the port. Apart from that the patch looks good.
> > >>
> > >> I admit I have barely copied this from Gen-2 boards DTS, but reading
> > >> the driver code and binding description again, I think this is
> > >> correct, as the output port numbering and mandatory input port (which
> > >> is missing here) only apply to adv7180cp/st chip versions.
> > >>
> > >> Here we describe plain adv7180, no need to number output ports afaict.
> > >
> > > Indeed, my bad.
> > >
> > > Shouldn't you however use "adi,adv7180cp" as that's the chip we are using
> > > ?
> > > The "adi,adv7180" has no port documented in its DT bindings, so it
> > > shouldn't have any port node at all.
> >
> > I'm a bit confused here.
> >
> > The only Gen-2 board using the "adi,adv7180cp" compatible string is
> > Gose, which is also the only one I can get schematics for. That board
> > indeed feature an ADV7180WBCP32Z, as the Draak does. I cannot get
> > schematics for any other Gen-2 board, to compare the ADV7180 variant
> > installed there. If anyone can confirm that all other Gen-2 board uses
> > a different version (or that all other Gen-2 board DTS file use a
> > wrong compatible string value), I'll re-send this with a different
> > compatible value and proper port nodes numbering.
>
> Other Gen2 boards use a ADV7180WBCP32Z as well. The issue here isn't that the
> chip you're trying to support is different. The DT bindings that were
> initially written for the adi,adv7180 didn't have port nodes. When it was time
> to add them, we realized that two variants of the chip existed with different
> connectivity. We have thus added two new compatible strings to differentiate
> them, with different port numbers. The old compatible string should be
> deprecated in favour of the new ones.
>

Ack, thanks for explaining.

I don't have any Gen2 board here, but if someone is willing to test, I
can change the Gen2 bindings to use the new version in a follow-up
series.

Thanks
   j

> > >>>> +			/*
> > >>>> +			 * The VIN4 video input path is shared between
> > >>>> +			 * CVBS and HDMI inputs through SW[49-54] switches.
> > >>>> +			 *
> > >>>> +			 * CVBS is the default selection, link it to VIN4 here.
> > >>>> +			 */
> > >>>> +			adv7180_out: endpoint {
> > >>>> +				remote-endpoint = <&vin4_in>;
> > >>>> +			};
> > >>>> +		};
> > >>>> +	};
> > >>>>  };
> > >>>>
> > >>>>  &i2c1 {
> > >>>> @@ -246,3 +268,23 @@
> > >>>>  	timeout-sec = <60>;
> > >>>>  	status = "okay";
> > >>>>  };
>  >>>> +
> > >>>> +&vin4 {
> > >>>> +	pinctrl-0 = <&vin4_pins_cvbs>;
> > >>>> +	pinctrl-names = "default";
> > >>>> +
> > >>>> +	status = "okay";
> > >>>> +
> > >>>> +	ports {
> > >>>> +		#address-cells = <1>;
> > >>>> +		#size-cells = <0>;
> > >>>> +
> > >>>> +		port at 0 {
> > >>>> +			reg = <0>;
> > >>>> +
> > >>>> +			vin4_in: endpoint {
> > >>>> +				remote-endpoint = <&adv7180_out>;
> > >>>> +			};
> > >>>> +		};
> > >>>> +	};
> > >>>> +};
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/7c99b1ef/attachment.sig>

^ permalink raw reply

* [reset-control] How to initialize hardware state with the shared reset line?
From: Martin Blumenstingl @ 2018-05-24 20:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526997879.3671.27.camel@pengutronix.de>

Hi Philipp,

On Tue, May 22, 2018 at 4:04 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Hi Martin,
>
> On Mon, 2018-05-21 at 12:40 +0200, Martin Blumenstingl wrote:
>> Hello,
>>
>> On Mon, May 21, 2018 at 3:27 AM, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>> > Hi.
>> >
>> >
>> > 2018-05-20 19:57 GMT+09:00 Martin Blumenstingl
>> > <martin.blumenstingl@googlemail.com>:
>> > > Hi,
>> > >
>> > > On Thu, May 10, 2018 at 11:16 AM, Masahiro Yamada
>> > > <yamada.masahiro@socionext.com> wrote:
>> > > [snip]
>> > > > I may be missing something, but
>> > > > one solution might be reset hogging on the
>> > > > reset provider side.  This allows us to describe
>> > > > the initial state of reset lines in the reset controller.
>> > > >
>> > > > The idea for "reset-hog" is similar to:
>> > > >  - "gpio-hog" defined in
>> > > >    Documentation/devicetree/bindings/gpio/gpio.txt
>> > > >  - "assigned-clocks" defined in
>> > > >    Documetation/devicetree/bindings/clock/clock-bindings.txt
>> > > >
>> > > >
>> > > >
>> > > > For example,
>> > > >
>> > > >    reset-controller {
>> > > >             ....
>> > > >
>> > > >             line_a {
>> > > >                   reset-hog;
>> > > >                   resets = <1>;
>> > > >                   reset-assert;
>> > > >             };
>> > > >    }
>> > > >
>> > > >
>> > > > When the reset controller is registered,
>> > > > the reset ID '1' is asserted.
>> > > >
>> > > >
>> > > > So, all reset consumers that share the reset line '1'
>> > > > will start from the asserted state
>> > > > (i.e. defined state machine state).
>> > >
>> > > I wonder if a "reset hog" can be board specific:
>> > > - GPIO hogs are definitely board specific (meson-gxbb-odroidc2.dts for
>> > > example uses it to take the USB hub out of reset)
>> > > - assigned-clock-parents (and the like) can also be board specific (I
>> > > made up a use-case since I don't know of any actual examples: board A
>> > > uses an external XTAL while board B uses some other internal
>> > > clock-source because it doesn't have an external XTAL)
>> > >
>> > > however, can reset lines be board specific? or in other words: do we
>> > > need to describe them in device-tree?
>> >
>> > Indeed.
>> >
>> > I did not come up with board-specific cases.
>> >
>> > The problem we are discussing is SoC-specific,
>> > and reset-controller drivers are definitely SoC-specific.
>> >
>> > So, I think the initial state can be coded in drivers instead of DT.
>>
>> OK, let's also hear Philipp's (reset framework maintainer) opinion on this
>
> I'd like to know if there are other SoC families besides Amlogic Meson
> that potentially could have this problem and about how many of the
> resets that are documented in include/dt-bindings/reset/amlogic,meson*
> we are actually talking. Are all of those initially deasserted and none
> of the connected peripherals have power-on reset mechanisms?
I cannot speak for other SoC families besides Amlogic
Meson8/Meson8b/Meson8m2 and GX (disclaimer: I am a community
contributor, I don't have access to Amlogic's internal datasheets - my
knowledge is based on their public datasheets, their GPL kernel/u-boot
sources and trial and error)

it seems that at least "some" (but I don't know the exact number)
resets are de-asserted by the bootloader
Amlogic's u-boot for example also enables all gate clocks by default

I CC'ed the Amlogic mailing list because I'm not sure if everyone
working on that SoC family is watching the linux-arm-kernel mailing
list

>> > > we could extend struct reset_controller_dev (= reset controller
>> > > driver) if they are not board specific:
>> > > - either assert all reset lines by default except if they are listed
>> > > in a new field (may break backwards compatibility, requires testing of
>> > > all reset controller drivers)
>> >
>> > This is quite simple, but I am afraid there are some cases where the forcible
>> > reset-assert is not preferred.
>> >
>> > For example, the earlycon.  When we use earlycon, we would expect it has been
>> > initialized by a boot-loader, or something.
>> > If it is reset-asserted on the while, the console output
>> > will not be good.
>>
>> indeed, so let's skip this idea
>
> Maybe we should at first add initial reset assertion to the Meson driver
> on a case by case bases?
this seems simple enough to test it - we can still generalize this
later on (either by adding support to the reset framework, DT bindings
or something else)

> We can't add required reset hog DT bindings to the Meson reset
> controller anyway without breaking DT backwards compatibility.
>
>> > > - specify a list of reset lines and their desired state (or to keep it
>> > > easy: specify a list of reset lines that should be asserted)
>> > > (I must admit that this is basically your idea but the definition is
>> > > moved from device-tree to the reset controller driver)
>> >
>> > Yes, I think the list of "reset line ID" and "init state" pairs
>> > would be nicer.
>>
>> $ grep -R "of_reset_n_cells = [^1]" drivers/reset/
>> drivers/reset/reset-berlin.c:   priv->rcdev.of_reset_n_cells = 2;
>> drivers/reset/hisilicon/reset-hi3660.c: rc->rst.of_reset_n_cells = 2;
>> drivers/reset/reset-ti-sci.c:   data->rcdev.of_reset_n_cells = 2;
>> drivers/reset/reset-lantiq.c:   priv->rcdev.of_reset_n_cells = 2;
>>
>> everything else uses only one reset cell
>> from the lantiq reset dt-binding documentation: "The first cell takes
>> the reset set bit and the second cell takes the status bit."
>>
>> I'm not sure what to do with drivers that specify != 1 reset-cell
>> though if we use a simple "init state pair"
>> maybe Philipp can share his opinion on this one as well
>
> See above, so far I am not convinced (either way) whether this should be
> described in the DT at all.
>
>> > > any "chip" specific differences could be expressed by using a
>> > > different of_device_id
>> > >
>> > > one the other hand: your "reset hog" solution looks fine to me if
>> > > reset lines can be board specific
>> > >
>> > > > From the discussion with Martin Blumenstingl
>> > > > (https://lkml.org/lkml/2018/4/28/115),
>> > > > the problem for Amlogic is that
>> > > > the reset line is "de-asserted" by default.
>> > > > If so, the 'reset-hog' would fix the problem,
>> > > > and DWC3 driver would be able to use
>> > > > shared, level reset, I think.
>> > >
>> > > I think you are right: if we could control the initial state then we
>> > > should be able to use level resets
>> >
>> >
>> > Even further, can we drop the shared reset_control_reset() support, maybe?
>> > (in other words, revert commit 7da33a37b48f11)
>>
>> I believe we need to keep this if there's hardware out there:
>> - where the reset controller only supports reset pulses
>> - at least one reset line is shared between multiple devices
>>
>> I didn't have a closer look at the Amlogic Meson6 SoC yet, but I think
>> it matches above criteria. as far as I know:
>> - the USB situation there is similar to Meson8b (USB controllers and
>> PHYs share a reset line)
>> - it uses an older reset controller IP block which does not support
>> level resets (only reset pulses)
>
> See my answer to Masahiro's first mail. I think somebody suggested in
> the past to add a fallback from the deassert to the reset op. I think
> this is something that should work in this case.
this is an interesting idea - it should work for Meson6 (in case
mainline ever gains support for this old SoC)


Regards
Martin

^ permalink raw reply

* uImage target support on arm64
From: Ramon Fried @ 2018-05-24 20:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524193449.jql27apudwaevwpf@tarshish>

On Thu, May 24, 2018 at 10:34 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> Hi Ramon,
>
> On Thu, May 24, 2018 at 10:05:15PM +0300, Ramon Fried wrote:
>> I've noticed that it's not supported.
>> Is it on purpose ?
>
> Yes. The 32bit load address in the uImage header in pretty limited when
> applied to 64bit ARM64. Even for ARM zImage is the preferred kernel format for
> quite some time now, since it allows flexible load address, as well as
> multi-platform kernels.
Hi Baruch.
I though that in terms of U-boot, the new FIT image is the preferred
kernel format.
Thanks,
Ramon.
>
>> If not I'll be happy to add support for: make uImage
>> for arm64 targets.
>
> baruch
>
> --
>      http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
>    - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

^ permalink raw reply

* [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Rob Herring @ 2018-05-24 20:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524181834.GF4828@sirena.org.uk>

On Thu, May 24, 2018 at 1:18 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>
> Should userspace have some involvement in this decision?  It knows if
> it's got any intention of loading modules for example.  Kernel config
> checks might be good enough, though it's going to be a pain to work out
> if the relevant driver is built as a module for example.

I looked into whether we could hook into uevents in some way. If we
knew when all the coldplug events had been handled, that would be
sufficient. But it doesn't look to me like we can tell when that
happens with the uevent netlink socket. I think about the only thing
we can tell is if userspace has opened a socket. I'm not all that
familiar with how the whole sequence works, so other opinions on this
would be helpful.

Also, for this to work with serial consoles, we have to make the
decision before we get to userspace. I couldn't get systemd to create
serial gettys either if we deferred later. There's some dependence on
/dev/console, but I didn't get to the bottom of it.

Rob

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox