Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] clk: aspeed: Move structures to header
From: Stephen Boyd @ 2019-08-16 17:01 UTC (permalink / raw)
  To: Joel Stanley, Michael Turquette
  Cc: Ryan Chen, linux-aspeed, Andrew Jeffery, linux-kernel,
	Rob Herring, linux-clk, linux-arm-kernel
In-Reply-To: <20190816155806.22869-2-joel@jms.id.au>

Quoting Joel Stanley (2019-08-16 08:58:05)
> diff --git a/drivers/clk/clk-aspeed.h b/drivers/clk/clk-aspeed.h
> new file mode 100644
> index 000000000000..92d384367c25
> --- /dev/null
> +++ b/drivers/clk/clk-aspeed.h
> @@ -0,0 +1,74 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Structures used by ASPEED clock drivers
> + *
> + * Copyright 2019 IBM Corp.
> + */

Please include reset.h (or whatever defines reset_controller_dev),
clk-provider.h, kernel.h (for container_of and types), and forward
declare struct regmap and clk_div_table here.

> +
> +/**
> + * struct aspeed_gate_data - Aspeed gated clocks

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [Freedreno] [PATCH v3 0/2] iommu/arm-smmu: Split pagetable support
From: Robin Murphy @ 2019-08-16 16:58 UTC (permalink / raw)
  To: freedreno, Rob Herring, Will Deacon, jean-philippe.brucker,
	linux-arm-msm, Joerg Roedel, linux-kernel, iommu, Zhen Lei,
	linux-arm-kernel
In-Reply-To: <20190815153304.GD28465@jcrouse1-lnx.qualcomm.com>

Hi Jordan,

On 15/08/2019 16:33, Jordan Crouse wrote:
> On Wed, Aug 07, 2019 at 04:21:38PM -0600, Jordan Crouse wrote:
>> (Sigh, resend. I freaked out my SMTP server)
>>
>> This is part of an ongoing evolution for enabling split pagetable support for
>> arm-smmu. Previous versions can be found [1].
>>
>> In the discussion for v2 Robin pointed out that this is a very Adreno specific
>> use case and that is exactly true. Not only do we want to configure and use a
>> pagetable in the TTBR1 space, we also want to configure the TTBR0 region but
>> not allocate a pagetable for it or touch it until the GPU hardware does so. As
>> much as I want it to be a generic concept it really isn't.
>>
>> This revision leans into that idea. Most of the same io-pgtable code is there
>> but now it is wrapped as an Adreno GPU specific format that is selected by the
>> compatible string in the arm-smmu device.
>>
>> Additionally, per Robin's suggestion we are skipping creating a TTBR0 pagetable
>> to save on wasted memory.
>>
>> This isn't as clean as I would like it to be but I think that this is a better
>> direction than trying to pretend that the generic format would work.
>>
>> I'm tempting fate by posting this and then taking some time off, but I wanted
>> to try to kick off a conversation or at least get some flames so I can try to
>> refine this again next week. Please take a look and give some advice on the
>> direction.
> 
> Will, Robin -
> 
> Modulo the impl changes from Robin, do you think that using a dedicated
> pagetable format is the right approach for supporting split pagetables for the
> Adreno GPU?

How many different Adreno drivers would benefit from sharing it?

The more I come back to this, the more I'm convinced that io-pgtable 
should focus on the heavy lifting of pagetable management - the code 
that nobody wants to have to write at all, let alone more than once - 
and any subtleties which aren't essential to that should be pushed back 
into whichever callers actually care. Consider that already, literally 
no caller actually uses an unmodified stage 1 TCR value as provided in 
the io_pgtable_cfg.

I feel it would be most productive to elaborate further in the form of 
patches, so let me get right on that and try to bash something out 
before I go home tonight...

Robin.

> If so, then is adding the changes to io-pgtable-arm.c possible for 5.4 and then
> add the implementation specific code on top of Robin's stack later or do you
> feel they should come as part of a package deal?
> 
> Jordan
> 
>> Jordan Crouse (2):
>>    iommu/io-pgtable-arm: Add support for ARM_ADRENO_GPU_LPAE io-pgtable
>>      format
>>    iommu/arm-smmu: Add support for Adreno GPU pagetable formats
>>
>>   drivers/iommu/arm-smmu.c       |   8 +-
>>   drivers/iommu/io-pgtable-arm.c | 214 ++++++++++++++++++++++++++++++++++++++---
>>   drivers/iommu/io-pgtable.c     |   1 +
>>   include/linux/io-pgtable.h     |   2 +
>>   4 files changed, 209 insertions(+), 16 deletions(-)
>>
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> Freedreno mailing list
>> Freedreno@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/freedreno
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 03/11] xen/arm: pass one less argument to dma_cache_maint
From: Christoph Hellwig @ 2019-08-16 16:43 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, x86, linux-kernel,
	iommu, xen-devel, Christoph Hellwig, linux-arm-kernel
In-Reply-To: <8585fb27-14e0-888c-6749-6862b4e16418@arm.com>

On Fri, Aug 16, 2019 at 02:37:58PM +0100, Robin Murphy wrote:
> On 16/08/2019 14:00, Christoph Hellwig wrote:
>> Instead of taking apart the dma address in both callers do it inside
>> dma_cache_maint itself.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> ---
>>   arch/arm/xen/mm.c | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
>> index 90574d89d0d4..d9da24fda2f7 100644
>> --- a/arch/arm/xen/mm.c
>> +++ b/arch/arm/xen/mm.c
>> @@ -43,13 +43,15 @@ static bool hypercall_cflush = false;
>>     /* functions called by SWIOTLB */
>>   -static void dma_cache_maint(dma_addr_t handle, unsigned long offset,
>> -	size_t size, enum dma_data_direction dir, enum dma_cache_op op)
>> +static void dma_cache_maint(dma_addr_t handle, size_t size,
>> +		enum dma_data_direction dir, enum dma_cache_op op)
>>   {
>>   	struct gnttab_cache_flush cflush;
>>   	unsigned long xen_pfn;
>> +	unsigned long offset = handle & ~PAGE_MASK;
>>   	size_t left = size;
>>   +	offset &= PAGE_MASK;
>
> Ahem... presumably that should be handle, not offset.

Ooops, yes.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: ftrace: Ensure module ftrace trampoline is coherent with I-side
From: Catalin Marinas @ 2019-08-16 16:40 UTC (permalink / raw)
  To: Will Deacon; +Cc: James Morse, stable, linux-arm-kernel, Ard Biesheuvel
In-Reply-To: <20190816135743.13683-1-will@kernel.org>

On Fri, Aug 16, 2019 at 02:57:43PM +0100, Will Deacon wrote:
> The initial support for dynamic ftrace trampolines in modules made use
> of an indirect branch which loaded its target from the beginning of
> a special section (e71a4e1bebaf7 ("arm64: ftrace: add support for far
> branches to dynamic ftrace")). Since no instructions were being patched,
> no cache maintenance was needed. However, later in be0f272bfc83 ("arm64:
> ftrace: emit ftrace-mod.o contents through code") this code was reworked
> to output the trampoline instructions directly into the PLT entry but,
> unfortunately, the necessary cache maintenance was overlooked.
> 
> Add a call to __flush_icache_range() after writing the new trampoline
> instructions but before patching in the branch to the trampoline.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: <stable@vger.kernel.org>
> Fixes: be0f272bfc83 ("arm64: ftrace: emit ftrace-mod.o contents through code")
> Signed-off-by: Will Deacon <will@kernel.org>

Queued for 5.3. Thanks.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [GIT PULL 2/3] ARM: samsung: mach for v5.4
From: Krzysztof Kozlowski @ 2019-08-16 16:30 UTC (permalink / raw)
  To: Olof Johansson, Arnd Bergmann, arm, soc
  Cc: linux-samsung-soc, Kukjin Kim, linux-arm-kernel,
	Krzysztof Kozlowski, linux-kernel
In-Reply-To: <20190816163042.6604-1-krzk@kernel.org>

The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:

  Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git tags/samsung-soc-5.4

for you to fetch changes up to 1fa70c7f49132513fb0da4afa7643395eedc7d35:

  ARM: exynos: Enable exynos-chipid driver (2019-08-15 20:29:58 +0200)

----------------------------------------------------------------
Samsung mach/soc changes for v5.4

1. Minor fixup in plat code (S3C platforms),
2. Enable exynos-chipid driver to provide SoC related information.

----------------------------------------------------------------
Linus Walleij (1):
      ARM: samsung: Include GPIO driver header

Pankaj Dubey (1):
      ARM: exynos: Enable exynos-chipid driver

 arch/arm/mach-exynos/Kconfig                   | 1 +
 arch/arm/plat-samsung/include/plat/gpio-core.h | 1 +
 2 files changed, 2 insertions(+)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [GIT PULL 3/3] ARM: dts: exynos: DT for v5.4
From: Krzysztof Kozlowski @ 2019-08-16 16:30 UTC (permalink / raw)
  To: Olof Johansson, Arnd Bergmann, arm, soc
  Cc: linux-samsung-soc, Kukjin Kim, linux-arm-kernel,
	Krzysztof Kozlowski, linux-kernel
In-Reply-To: <20190816163042.6604-1-krzk@kernel.org>

The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:

  Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)

are available in the Git repository at:

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

for you to fetch changes up to bfb77169306d5d560a8b62eebaf6d69d02e8d152:

  ARM: dts: exynos: Add CAM power domain to Exynos5422/5800 (2019-08-12 19:02:59 +0200)

----------------------------------------------------------------
Samsung DTS ARM changes for v5.4

1. Add AHCI to Exynos5250,
2. Add camera and GPU power domains to Exynos5422,
3. Minor cleanup.

----------------------------------------------------------------
Krzysztof Kozlowski (1):
      ARM: dts: exynos: Use space after '=' in exynos4412-itop-scp-core

Marek Szyprowski (4):
      ARM: dts: exynos: Add port map to Exynos5250 AHCI node
      ARM: dts: exynos: Move MSC power domain to the right (sorted) place
      ARM: dts: exynos: Add G3D power domain to Exynos542x
      ARM: dts: exynos: Add CAM power domain to Exynos5422/5800

 arch/arm/boot/dts/exynos4412-itop-scp-core.dtsi | 18 +++++++++---------
 arch/arm/boot/dts/exynos5250.dtsi               |  1 +
 arch/arm/boot/dts/exynos5420.dtsi               | 13 ++++++++++---
 arch/arm/boot/dts/exynos5800.dtsi               |  9 +++++++++
 4 files changed, 29 insertions(+), 12 deletions(-)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [GIT PULL 1/3] soc: samsung: Exynos for v5.4
From: Krzysztof Kozlowski @ 2019-08-16 16:30 UTC (permalink / raw)
  To: Olof Johansson, Arnd Bergmann, arm, soc
  Cc: linux-samsung-soc, Kukjin Kim, linux-arm-kernel,
	Krzysztof Kozlowski, linux-kernel

The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:

  Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git tags/samsung-drivers-5.4

for you to fetch changes up to 40d8aff614f71ab3cab20785b4f213e3802d4e87:

  soc: samsung: chipid: Convert exynos-chipid driver to use the regmap API (2019-08-15 20:25:25 +0200)

----------------------------------------------------------------
Samsung soc drivers changes for v5.4

Add Exynos Chipid driver for identification of product IDs and SoC
revisions.  The driver also exposes chipid regmap, later to be used by
Exynos Adaptive Supply Voltage driver (adjusting voltages to different
revisions of same SoC).

----------------------------------------------------------------
Pankaj Dubey (1):
      soc: samsung: Add exynos chipid driver support

Sylwester Nawrocki (1):
      soc: samsung: chipid: Convert exynos-chipid driver to use the regmap API

 drivers/soc/samsung/Kconfig               |   5 ++
 drivers/soc/samsung/Makefile              |   2 +
 drivers/soc/samsung/exynos-chipid.c       | 101 ++++++++++++++++++++++++++++++
 include/linux/soc/samsung/exynos-chipid.h |  52 +++++++++++++++
 4 files changed, 160 insertions(+)
 create mode 100644 drivers/soc/samsung/exynos-chipid.c
 create mode 100644 include/linux/soc/samsung/exynos-chipid.h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 2/2] watchdog: aspeed: Add support for AST2600
From: Guenter Roeck @ 2019-08-16 16:15 UTC (permalink / raw)
  To: Joel Stanley
  Cc: devicetree, Ryan Chen, linux-watchdog, linux-aspeed,
	Andrew Jeffery, Rob Herring, Wim Van Sebroeck, linux-arm-kernel
In-Reply-To: <20190816160347.23393-3-joel@jms.id.au>

On Sat, Aug 17, 2019 at 01:33:47AM +0930, Joel Stanley wrote:
> From: Ryan Chen <ryan_chen@aspeedtech.com>
> 
> The ast2600 can be supported by the same code as the ast2500.
> 
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  drivers/watchdog/aspeed_wdt.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index cc71861e033a..94f73796ba9d 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -31,9 +31,14 @@ static const struct aspeed_wdt_config ast2500_config = {
>  	.ext_pulse_width_mask = 0xfffff,
>  };
>  
> +static const struct aspeed_wdt_config ast2600_config = {
> +	.ext_pulse_width_mask = 0xfffff,
> +};
> +

Why not just reuse ast2500_config ?

Guenter

>  static const struct of_device_id aspeed_wdt_of_table[] = {
>  	{ .compatible = "aspeed,ast2400-wdt", .data = &ast2400_config },
>  	{ .compatible = "aspeed,ast2500-wdt", .data = &ast2500_config },
> +	{ .compatible = "aspeed,ast2600-wdt", .data = &ast2600_config },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
> @@ -259,7 +264,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>  		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
>  	}
>  
> -	if (of_device_is_compatible(np, "aspeed,ast2500-wdt")) {
> +	if ((of_device_is_compatible(np, "aspeed,ast2500-wdt")) ||
> +		(of_device_is_compatible(np, "aspeed,ast2600-wdt"))) {
>  		u32 reg = readl(wdt->base + WDT_RESET_WIDTH);
>  
>  		reg &= config->ext_pulse_width_mask;
> -- 
> 2.23.0.rc1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/7] [RFC] ARM: remove Intel iop33x and iop13xx support
From: Aaro Koskinen @ 2019-08-16 16:15 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Vinod Koul, Peter Teichmann, Arnd Bergmann, Bartosz Golaszewski,
	Linus Walleij, Linux Kernel Mailing List,
	open list:GPIO SUBSYSTEM, soc, linux-i2c, dmaengine, Dan Williams,
	Martin Michlmayr, Linux ARM
In-Reply-To: <20190816155833.GL13294@shell.armlinux.org.uk>

Hi,

On Fri, Aug 16, 2019 at 04:58:33PM +0100, Russell King - ARM Linux admin wrote:
> On Fri, Aug 16, 2019 at 06:42:49PM +0300, Aaro Koskinen wrote:
> > On Wed, Aug 14, 2019 at 10:36:01AM +0200, Linus Walleij wrote:
> > > On Mon, Aug 12, 2019 at 11:45 AM Martin Michlmayr <tbm@cyrius.com> wrote:
> > > > As Arnd points out, Debian used to have support for various iop32x
> > > > devices.  While Debian hasn't supported iop32x in a number of years,
> > > > these devices are still usable and in use (RMK being a prime example).
> > > 
> > > I suppose it could be a good idea to add support for iop32x to
> > > OpenWrt and/or OpenEmbedded, both of which support some
> > > pretty constrained systems.
> > 
> > This platform is not really too constrained... E.g. on N2100 you have
> > 512 MB RAM, SATA disks and gigabit ethernet. Not that different from
> > mvebu that Debian currently (?) supports. Maybe with multiplatform they
> > could support iop32x again.
> 
> Probably not.  The kernel has a dividing line between ARMv5 and ARMv6
> where it's not possible to multiplatform across that boundary, so
> you're already needing separate kernel images there.
> 
> Secondly, armhf distros won't be compatible with ARMv5, and to make
> them compatible will make performance on armhf suffer - you have to
> stop using barriers, exclusive load/store and a few other things.
> You have to rely on the kuser page exported by the kernel (which is
> now optional as it's deemed to be a security issue for ROP attacks)
> for some things that such a userspace requires - such as NPTL support.
> 
> Effectively, ARMv5 is an entirely separate userspace distro from armhf.

I thought they still had armel for ARMv5 and mvebu (kirkwood).

A.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2] ARM: dts: add device tree for Mecer Xtreme Mini S6
From: Heiko Stuebner @ 2019-08-16 16:07 UTC (permalink / raw)
  To: Justin Swartz
  Cc: Mark Rutland, devicetree, linux-kernel, linux-rockchip,
	Rob Herring, linux-arm-kernel
In-Reply-To: <20190811230015.28349-1-justin.swartz@risingedge.co.za>

Am Montag, 12. August 2019, 01:00:13 CEST schrieb Justin Swartz:
> The Mecer Xtreme Mini S6 features a Rockchip RK3229 SoC,
> 1GB DDR3 RAM, 8GB eMMC, MicroSD port, 10/100Mbps Ethernet,
> Realtek 8723BS WLAN module, 2 x USB 2.0 ports, HDMI output,
> and S/PDIF output.
> 
> Signed-off-by: Justin Swartz <justin.swartz@risingedge.co.za>

applied for 5.4

Thanks
Heiko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 2/2] watchdog: aspeed: Add support for AST2600
From: Joel Stanley @ 2019-08-16 16:03 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Rob Herring
  Cc: devicetree, Ryan Chen, linux-watchdog, linux-aspeed,
	Andrew Jeffery, linux-arm-kernel
In-Reply-To: <20190816160347.23393-1-joel@jms.id.au>

From: Ryan Chen <ryan_chen@aspeedtech.com>

The ast2600 can be supported by the same code as the ast2500.

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/watchdog/aspeed_wdt.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
index cc71861e033a..94f73796ba9d 100644
--- a/drivers/watchdog/aspeed_wdt.c
+++ b/drivers/watchdog/aspeed_wdt.c
@@ -31,9 +31,14 @@ static const struct aspeed_wdt_config ast2500_config = {
 	.ext_pulse_width_mask = 0xfffff,
 };
 
+static const struct aspeed_wdt_config ast2600_config = {
+	.ext_pulse_width_mask = 0xfffff,
+};
+
 static const struct of_device_id aspeed_wdt_of_table[] = {
 	{ .compatible = "aspeed,ast2400-wdt", .data = &ast2400_config },
 	{ .compatible = "aspeed,ast2500-wdt", .data = &ast2500_config },
+	{ .compatible = "aspeed,ast2600-wdt", .data = &ast2600_config },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
@@ -259,7 +264,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
 		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
 	}
 
-	if (of_device_is_compatible(np, "aspeed,ast2500-wdt")) {
+	if ((of_device_is_compatible(np, "aspeed,ast2500-wdt")) ||
+		(of_device_is_compatible(np, "aspeed,ast2600-wdt"))) {
 		u32 reg = readl(wdt->base + WDT_RESET_WIDTH);
 
 		reg &= config->ext_pulse_width_mask;
-- 
2.23.0.rc1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 1/2] dt-bindings: watchdog: Add ast2600 compatible
From: Joel Stanley @ 2019-08-16 16:03 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Rob Herring
  Cc: devicetree, Ryan Chen, linux-watchdog, linux-aspeed,
	Andrew Jeffery, linux-arm-kernel
In-Reply-To: <20190816160347.23393-1-joel@jms.id.au>

This adds a compatible for the ast2600, a new ASPEED SoC.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
index c5077a1f5cb3..d78d4a8fb868 100644
--- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
@@ -4,6 +4,7 @@ Required properties:
  - compatible: must be one of:
 	- "aspeed,ast2400-wdt"
 	- "aspeed,ast2500-wdt"
+	- "aspeed,ast2600-wdt"
 
  - reg: physical base address of the controller and length of memory mapped
    region
-- 
2.23.0.rc1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 0/2] watchdog: aspeed: Add support for ast2600
From: Joel Stanley @ 2019-08-16 16:03 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Rob Herring
  Cc: devicetree, Ryan Chen, linux-watchdog, linux-aspeed,
	Andrew Jeffery, linux-arm-kernel

Hello,

Here's a small patch series to enable the ast2600 watchdog.

Joel Stanley (1):
  dt-bindings: watchdog: Add ast2600 compatible

Ryan Chen (1):
  watchdog: aspeed: Add support for AST2600

 Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 1 +
 drivers/watchdog/aspeed_wdt.c                             | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

-- 
2.23.0.rc1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/7] [RFC] ARM: remove Intel iop33x and iop13xx support
From: Russell King - ARM Linux admin @ 2019-08-16 15:58 UTC (permalink / raw)
  To: Aaro Koskinen
  Cc: Vinod Koul, Peter Teichmann, Arnd Bergmann, Bartosz Golaszewski,
	Linus Walleij, Linux Kernel Mailing List,
	open list:GPIO SUBSYSTEM, soc, linux-i2c, dmaengine, Dan Williams,
	Martin Michlmayr, Linux ARM
In-Reply-To: <20190816154249.GA30291@darkstar.musicnaut.iki.fi>

On Fri, Aug 16, 2019 at 06:42:49PM +0300, Aaro Koskinen wrote:
> Hi,
> 
> On Wed, Aug 14, 2019 at 10:36:01AM +0200, Linus Walleij wrote:
> > On Mon, Aug 12, 2019 at 11:45 AM Martin Michlmayr <tbm@cyrius.com> wrote:
> > > As Arnd points out, Debian used to have support for various iop32x
> > > devices.  While Debian hasn't supported iop32x in a number of years,
> > > these devices are still usable and in use (RMK being a prime example).
> > 
> > I suppose it could be a good idea to add support for iop32x to
> > OpenWrt and/or OpenEmbedded, both of which support some
> > pretty constrained systems.
> 
> This platform is not really too constrained... E.g. on N2100 you have
> 512 MB RAM, SATA disks and gigabit ethernet. Not that different from
> mvebu that Debian currently (?) supports. Maybe with multiplatform they
> could support iop32x again.

Probably not.  The kernel has a dividing line between ARMv5 and ARMv6
where it's not possible to multiplatform across that boundary, so
you're already needing separate kernel images there.

Secondly, armhf distros won't be compatible with ARMv5, and to make
them compatible will make performance on armhf suffer - you have to
stop using barriers, exclusive load/store and a few other things.
You have to rely on the kuser page exported by the kernel (which is
now optional as it's deemed to be a security issue for ROP attacks)
for some things that such a userspace requires - such as NPTL support.

Effectively, ARMv5 is an entirely separate userspace distro from armhf.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 2/2] clk: Add support for AST2600 SoC
From: Joel Stanley @ 2019-08-16 15:58 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette
  Cc: Ryan Chen, linux-aspeed, Andrew Jeffery, linux-kernel,
	Rob Herring, linux-clk, linux-arm-kernel
In-Reply-To: <20190816155806.22869-1-joel@jms.id.au>

The ast2600 is a new BMC SoC from ASPEED. It contains many more clocks
than the previous iterations, so support is broken out into it's own
driver.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/clk/Makefile                      |   1 +
 drivers/clk/clk-ast2600.c                 | 701 ++++++++++++++++++++++
 include/dt-bindings/clock/ast2600-clock.h | 116 ++++
 3 files changed, 818 insertions(+)
 create mode 100644 drivers/clk/clk-ast2600.c
 create mode 100644 include/dt-bindings/clock/ast2600-clock.h

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 0cad76021297..0138fb14e6f8 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_ARCH_EFM32)		+= clk-efm32gg.o
 obj-$(CONFIG_COMMON_CLK_FIXED_MMIO)	+= clk-fixed-mmio.o
 obj-$(CONFIG_COMMON_CLK_GEMINI)		+= clk-gemini.o
 obj-$(CONFIG_COMMON_CLK_ASPEED)		+= clk-aspeed.o
+obj-$(CONFIG_MACH_ASPEED_G6)		+= clk-ast2600.o
 obj-$(CONFIG_ARCH_HIGHBANK)		+= clk-highbank.o
 obj-$(CONFIG_CLK_HSDK)			+= clk-hsdk-pll.o
 obj-$(CONFIG_COMMON_CLK_LOCHNAGAR)	+= clk-lochnagar.o
diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
new file mode 100644
index 000000000000..083d5299238c
--- /dev/null
+++ b/drivers/clk/clk-ast2600.c
@@ -0,0 +1,701 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright IBM Corp
+// Copyright ASPEED Technology
+
+#define pr_fmt(fmt) "clk-ast2600: " fmt
+
+#include <linux/clk-provider.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include <dt-bindings/clock/ast2600-clock.h>
+
+#include "clk-aspeed.h"
+
+#define ASPEED_G6_NUM_CLKS		69
+
+#define ASPEED_G6_SILICON_REV		0x004
+
+#define ASPEED_G6_RESET_CTRL		0x040
+#define ASPEED_G6_RESET_CTRL2		0x050
+
+#define ASPEED_G6_CLK_STOP_CTRL		0x080
+#define ASPEED_G6_CLK_STOP_CTRL2	0x090
+
+#define ASPEED_G6_MISC_CTRL		0x0C0
+#define  UART_DIV13_EN			BIT(12)
+
+#define ASPEED_G6_CLK_SELECTION1	0x300
+#define ASPEED_G6_CLK_SELECTION2	0x304
+#define ASPEED_G6_CLK_SELECTION4	0x310
+
+#define ASPEED_HPLL_PARAM		0x200
+#define ASPEED_APLL_PARAM		0x210
+#define ASPEED_MPLL_PARAM		0x220
+#define ASPEED_EPLL_PARAM		0x240
+#define ASPEED_DPLL_PARAM		0x260
+
+#define ASPEED_G6_STRAP1		0x500
+
+/* Globally visible clocks */
+static DEFINE_SPINLOCK(aspeed_clk_lock);
+
+/* Keeps track of all clocks */
+static struct clk_hw_onecell_data *aspeed_g6_clk_data;
+
+static void __iomem *scu_g6_base;
+
+static const struct aspeed_gate_data aspeed_g6_gates[] = {
+	/*				    clk rst  name		parent	 flags */
+	[ASPEED_CLK_GATE_MCLK]		= {  0, -1, "mclk-gate",	"mpll",	 CLK_IS_CRITICAL }, /* SDRAM */
+	[ASPEED_CLK_GATE_ECLK]		= {  1, -1, "eclk-gate",	"eclk",	 0 },	/* Video Engine */
+	[ASPEED_CLK_GATE_GCLK]		= {  2,  7, "gclk-gate",	NULL,	 0 },	/* 2D engine */
+	/* vclk parent - dclk/d1clk/hclk/mclk */
+	[ASPEED_CLK_GATE_VCLK]		= {  3,  6, "vclk-gate",	NULL,	 0 },	/* Video Capture */
+	[ASPEED_CLK_GATE_BCLK]		= {  4,  8, "bclk-gate",	"bclk",	 CLK_IS_CRITICAL }, /* PCIe/PCI */
+	/* From dpll */
+	[ASPEED_CLK_GATE_DCLK]		= {  5, -1, "dclk-gate",	NULL,	 CLK_IS_CRITICAL }, /* DAC */
+	[ASPEED_CLK_GATE_REF0CLK]	= {  6, -1, "ref0clk-gate",	"clkin", CLK_IS_CRITICAL },
+	[ASPEED_CLK_GATE_USBPORT2CLK]	= {  7,  3, "usb-port2-gate",	NULL,	 0 },	/* USB2.0 Host port 2 */
+	/* Reserved 8 */
+	[ASPEED_CLK_GATE_USBUHCICLK]	= {  9, 15, "usb-uhci-gate",	NULL,	 0 },	/* USB1.1 (requires port 2 enabled) */
+	/* From dpll/epll/40mhz usb p1 phy/gpioc6/dp phy pll */
+	[ASPEED_CLK_GATE_D1CLK]		= { 10, 13, "d1clk-gate",	"d1clk", 0 },	/* GFX CRT */
+	/* Reserved 11/12 */
+	[ASPEED_CLK_GATE_YCLK]		= { 13,  4, "yclk-gate",	NULL,	 0 },	/* HAC */
+	[ASPEED_CLK_GATE_USBPORT1CLK]	= { 14, 14, "usb-port1-gate",	NULL,	 0 },	/* USB2 hub/USB2 host port 1/USB1.1 dev */
+	[ASPEED_CLK_GATE_UART5CLK]	= { 15, -1, "uart5clk-gate",	"uart",	 0 },	/* UART5 */
+	/* Reserved 16/19 */
+	[ASPEED_CLK_GATE_MAC1CLK]	= { 20, 11, "mac1clk-gate",	"mac12", 0 },	/* MAC1 */
+	[ASPEED_CLK_GATE_MAC2CLK]	= { 21, 12, "mac2clk-gate",	"mac12", 0 },	/* MAC2 */
+	/* Reserved 22/23 */
+	[ASPEED_CLK_GATE_RSACLK]	= { 24,  4, "rsaclk-gate",	NULL,	 0 },	/* HAC */
+	[ASPEED_CLK_GATE_RVASCLK]	= { 25,  9, "rvasclk-gate",	NULL,	 0 },	/* RVAS */
+	/* Reserved 26 */
+	[ASPEED_CLK_GATE_EMMCCLK]	= { 27, 16, "emmcclk-gate",	NULL,	 0 },	/* For card clk */
+	/* Reserved 28/29/30 */
+	[ASPEED_CLK_GATE_LCLK]		= { 32, 32, "lclk-gate",	NULL,	 CLK_IS_CRITICAL }, /* LPC */
+	[ASPEED_CLK_GATE_ESPICLK]	= { 33, -1, "espiclk-gate",	NULL,	 CLK_IS_CRITICAL }, /* eSPI */
+	[ASPEED_CLK_GATE_REF1CLK]	= { 34, -1, "ref1clk-gate",	"clkin", CLK_IS_CRITICAL },
+	/* Reserved 35 */
+	[ASPEED_CLK_GATE_SDCLK]		= { 36, 56, "sdclk-gate",	NULL,	 0 },	/* SDIO/SD */
+	[ASPEED_CLK_GATE_LHCCLK]	= { 37, -1, "lhclk-gate",	"lhclk", 0 },	/* LPC master/LPC+ */
+	/* Reserved 38 RSA: no longer used */
+	/* Reserved 39 */
+	[ASPEED_CLK_GATE_I3C0CLK]	= { 40,  40, "i3c0clk-gate",	NULL,	 0 },	/* I3C0 */
+	[ASPEED_CLK_GATE_I3C1CLK]	= { 41,  41, "i3c1clk-gate",	NULL,	 0 },	/* I3C1 */
+	[ASPEED_CLK_GATE_I3C2CLK]	= { 42,  42, "i3c2clk-gate",	NULL,	 0 },	/* I3C2 */
+	[ASPEED_CLK_GATE_I3C3CLK]	= { 43,  43, "i3c3clk-gate",	NULL,	 0 },	/* I3C3 */
+	[ASPEED_CLK_GATE_I3C4CLK]	= { 44,  44, "i3c4clk-gate",	NULL,	 0 },	/* I3C4 */
+	[ASPEED_CLK_GATE_I3C5CLK]	= { 45,  45, "i3c5clk-gate",	NULL,	 0 },	/* I3C5 */
+	[ASPEED_CLK_GATE_I3C6CLK]	= { 46,  46, "i3c6clk-gate",	NULL,	 0 },	/* I3C6 */
+	[ASPEED_CLK_GATE_I3C7CLK]	= { 47,  47, "i3c7clk-gate",	NULL,	 0 },	/* I3C7 */
+	[ASPEED_CLK_GATE_UART1CLK]	= { 48,  -1, "uart1clk-gate",	"uart",	 0 },	/* UART1 */
+	[ASPEED_CLK_GATE_UART2CLK]	= { 49,  -1, "uart2clk-gate",	"uart",	 0 },	/* UART2 */
+	[ASPEED_CLK_GATE_UART3CLK]	= { 50,  -1, "uart3clk-gate",	"uart",  0 },	/* UART3 */
+	[ASPEED_CLK_GATE_UART4CLK]	= { 51,  -1, "uart4clk-gate",	"uart",	 0 },	/* UART4 */
+	[ASPEED_CLK_GATE_MAC3CLK]	= { 52,  52, "mac3clk-gate",	"mac34", 0 },	/* MAC3 */
+	[ASPEED_CLK_GATE_MAC4CLK]	= { 53,  53, "mac4clk-gate",	"mac34", 0 },	/* MAC4 */
+	[ASPEED_CLK_GATE_UART6CLK]	= { 54,  -1, "uart6clk-gate",	"uartx", 0 },	/* UART6 */
+	[ASPEED_CLK_GATE_UART7CLK]	= { 55,  -1, "uart7clk-gate",	"uartx", 0 },	/* UART7 */
+	[ASPEED_CLK_GATE_UART8CLK]	= { 56,  -1, "uart8clk-gate",	"uartx", 0 },	/* UART8 */
+	[ASPEED_CLK_GATE_UART9CLK]	= { 57,  -1, "uart9clk-gate",	"uartx", 0 },	/* UART9 */
+	[ASPEED_CLK_GATE_UART10CLK]	= { 58,  -1, "uart10clk-gate",	"uartx", 0 },	/* UART10 */
+	[ASPEED_CLK_GATE_UART11CLK]	= { 59,  -1, "uart11clk-gate",	"uartx", 0 },	/* UART11 */
+	[ASPEED_CLK_GATE_UART12CLK]	= { 60,  -1, "uart12clk-gate",	"uartx", 0 },	/* UART12 */
+	[ASPEED_CLK_GATE_UART13CLK]	= { 61,  -1, "uart13clk-gate",	"uartx", 0 },	/* UART13 */
+	[ASPEED_CLK_GATE_FSICLK]	= { 62,  59, "fsiclk-gate",	NULL,	 0 },	/* FSI */
+};
+
+static const char * const eclk_parent_names[] = { "mpll", "hpll", "dpll" };
+
+static const struct clk_div_table ast2600_eclk_div_table[] = {
+	{ 0x0, 2 },
+	{ 0x1, 2 },
+	{ 0x2, 3 },
+	{ 0x3, 4 },
+	{ 0x4, 5 },
+	{ 0x5, 6 },
+	{ 0x6, 7 },
+	{ 0x7, 8 },
+	{ 0 }
+};
+
+static const struct clk_div_table ast2600_mac_div_table[] = {
+	{ 0x0, 4 },
+	{ 0x1, 4 },
+	{ 0x2, 6 },
+	{ 0x3, 8 },
+	{ 0x4, 10 },
+	{ 0x5, 12 },
+	{ 0x6, 14 },
+	{ 0x7, 16 },
+	{ 0 }
+};
+
+static const struct clk_div_table ast2600_div_table[] = {
+	{ 0x0, 4 },
+	{ 0x1, 8 },
+	{ 0x2, 12 },
+	{ 0x3, 16 },
+	{ 0x4, 20 },
+	{ 0x5, 24 },
+	{ 0x6, 28 },
+	{ 0x7, 32 },
+	{ 0 }
+};
+
+/* For hpll/dpll/epll/mpll */
+static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)
+{
+	unsigned int mult, div;
+
+	if (val & BIT(24)) {
+		/* Pass through mode */
+		mult = div = 1;
+	} else {
+		/* F = 25Mhz * [(M + 2) / (n + 1)] / (p + 1) */
+		u32 m = val  & 0x1fff;
+		u32 n = (val >> 13) & 0x3f;
+		u32 p = (val >> 19) & 0xf;
+		mult = (m + 1) / (n + 1);
+		div = (p + 1);
+	}
+	return clk_hw_register_fixed_factor(NULL, name, "clkin", 0,
+			mult, div);
+};
+
+static struct clk_hw *ast2600_calc_apll(const char *name, u32 val)
+{
+	unsigned int mult, div;
+
+	if (val & BIT(20)) {
+		/* Pass through mode */
+		mult = div = 1;
+	} else {
+		/* F = 25Mhz * (2-od) * [(m + 2) / (n + 1)] */
+		u32 m = (val >> 5) & 0x3f;
+		u32 od = (val >> 4) & 0x1;
+		u32 n = val & 0xf;
+
+		mult = (2 - od) * (m + 2);
+		div = n + 1;
+	}
+	return clk_hw_register_fixed_factor(NULL, name, "clkin", 0,
+			mult, div);
+};
+
+static u32 get_bit(u8 idx)
+{
+	if (idx < 32)
+		return BIT(idx);
+	else
+		return BIT(idx - 32);
+}
+
+static u32 get_reset_reg(struct aspeed_clk_gate *gate)
+{
+	if (gate->reset_idx < 32)
+		return ASPEED_G6_RESET_CTRL;
+	else
+		return ASPEED_G6_RESET_CTRL2;
+}
+
+static u32 get_clock_reg(struct aspeed_clk_gate *gate)
+{
+	if (gate->clock_idx < 32)
+		return ASPEED_G6_CLK_STOP_CTRL;
+	else
+		return ASPEED_G6_CLK_STOP_CTRL2;
+}
+
+static int aspeed_g6_clk_is_enabled(struct clk_hw *hw)
+{
+	struct aspeed_clk_gate *gate = to_aspeed_clk_gate(hw);
+	u32 clk = get_bit(gate->clock_idx);
+	u32 rst = get_bit(gate->reset_idx);
+	u32 reg;
+	u32 enval;
+
+	/*
+	 * If the IP is in reset, treat the clock as not enabled,
+	 * this happens with some clocks such as the USB one when
+	 * coming from cold reset. Without this, aspeed_clk_enable()
+	 * will fail to lift the reset.
+	 */
+	if (gate->reset_idx >= 0) {
+		regmap_read(gate->map, get_reset_reg(gate), &reg);
+
+		if (reg & rst)
+			return 0;
+	}
+
+	regmap_read(gate->map, get_clock_reg(gate), &reg);
+
+	enval = (gate->flags & CLK_GATE_SET_TO_DISABLE) ? 0 : clk;
+
+	return ((reg & clk) == enval) ? 1 : 0;
+}
+
+static int aspeed_g6_clk_enable(struct clk_hw *hw)
+{
+	struct aspeed_clk_gate *gate = to_aspeed_clk_gate(hw);
+	unsigned long flags;
+	u32 clk = get_bit(gate->clock_idx);
+	u32 rst = get_bit(gate->reset_idx);
+
+	spin_lock_irqsave(gate->lock, flags);
+
+	if (aspeed_g6_clk_is_enabled(hw)) {
+		spin_unlock_irqrestore(gate->lock, flags);
+		return 0;
+	}
+
+	if (gate->reset_idx >= 0) {
+		/* Put IP in reset */
+		regmap_write(gate->map, get_reset_reg(gate), rst);
+		/* Delay 100us */
+		udelay(100);
+	}
+
+	/* Enable clock */
+	if (gate->flags & CLK_GATE_SET_TO_DISABLE)
+		regmap_write(gate->map, get_clock_reg(gate), clk);
+	else
+		/* Use set to clear register */
+		regmap_write(gate->map, get_clock_reg(gate) + 0x04, clk);
+
+	if (gate->reset_idx >= 0) {
+		/* A delay of 10ms is specified by the ASPEED docs */
+		mdelay(10);
+		/* Take IP out of reset */
+		regmap_write(gate->map, get_reset_reg(gate) + 0x4, rst);
+	}
+
+	spin_unlock_irqrestore(gate->lock, flags);
+
+	return 0;
+}
+
+static void aspeed_g6_clk_disable(struct clk_hw *hw)
+{
+	struct aspeed_clk_gate *gate = to_aspeed_clk_gate(hw);
+	unsigned long flags;
+	u32 clk = get_bit(gate->clock_idx);
+
+	spin_lock_irqsave(gate->lock, flags);
+
+	if (gate->flags & CLK_GATE_SET_TO_DISABLE)
+		regmap_write(gate->map, get_clock_reg(gate), clk);
+	else
+		/* Use set to clear register */
+		regmap_write(gate->map, get_clock_reg(gate) + 0x4, clk);
+
+	spin_unlock_irqrestore(gate->lock, flags);
+}
+
+static const struct clk_ops aspeed_g6_clk_gate_ops = {
+	.enable = aspeed_g6_clk_enable,
+	.disable = aspeed_g6_clk_disable,
+	.is_enabled = aspeed_g6_clk_is_enabled,
+};
+
+static int aspeed_g6_reset_deassert(struct reset_controller_dev *rcdev,
+				    unsigned long id)
+{
+	struct aspeed_reset *ar = to_aspeed_reset(rcdev);
+	u32 rst = get_bit(id);
+	u32 reg = id >= 32 ? ASPEED_G6_RESET_CTRL2 : ASPEED_G6_RESET_CTRL;
+
+	/* Use set to clear register */
+	return regmap_write(ar->map, reg + 0x04, rst);
+}
+
+static int aspeed_g6_reset_assert(struct reset_controller_dev *rcdev,
+				  unsigned long id)
+{
+	struct aspeed_reset *ar = to_aspeed_reset(rcdev);
+	u32 rst = get_bit(id);
+	u32 reg = id >= 32 ? ASPEED_G6_RESET_CTRL2 : ASPEED_G6_RESET_CTRL;
+
+	return regmap_write(ar->map, reg, rst);
+}
+
+static int aspeed_g6_reset_status(struct reset_controller_dev *rcdev,
+				  unsigned long id)
+{
+	struct aspeed_reset *ar = to_aspeed_reset(rcdev);
+	int ret;
+	u32 val;
+	u32 rst = get_bit(id);
+	u32 reg = id >= 32 ? ASPEED_G6_RESET_CTRL2 : ASPEED_G6_RESET_CTRL;
+
+	ret = regmap_read(ar->map, reg, &val);
+	if (ret)
+		return ret;
+
+	return !!(val & rst);
+}
+
+static const struct reset_control_ops aspeed_g6_reset_ops = {
+	.assert = aspeed_g6_reset_assert,
+	.deassert = aspeed_g6_reset_deassert,
+	.status = aspeed_g6_reset_status,
+};
+
+static struct clk_hw *aspeed_g6_clk_hw_register_gate(struct device *dev,
+		const char *name, const char *parent_name, unsigned long flags,
+		struct regmap *map, u8 clock_idx, u8 reset_idx,
+		u8 clk_gate_flags, spinlock_t *lock)
+{
+	struct aspeed_clk_gate *gate;
+	struct clk_init_data init;
+	struct clk_hw *hw;
+	int ret;
+
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &aspeed_g6_clk_gate_ops;
+	init.flags = flags;
+	init.parent_names = parent_name ? &parent_name : NULL;
+	init.num_parents = parent_name ? 1 : 0;
+
+	gate->map = map;
+	gate->clock_idx = clock_idx;
+	gate->reset_idx = reset_idx;
+	gate->flags = clk_gate_flags;
+	gate->lock = lock;
+	gate->hw.init = &init;
+
+	hw = &gate->hw;
+	ret = clk_hw_register(dev, hw);
+	if (ret) {
+		kfree(gate);
+		hw = ERR_PTR(ret);
+	}
+
+	return hw;
+}
+
+static const char * const vclk_parent_names[] = {
+	"dpll",
+	"d1pll",
+	"hclk",
+	"mclk",
+};
+
+static const char * const d1clk_parent_names[] = {
+	"dpll",
+	"epll",
+	"usb-phy-40m",
+	"gpioc6_clkin",
+	"dp_phy_pll",
+};
+
+static int aspeed_g6_clk_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct aspeed_reset *ar;
+	struct regmap *map;
+	struct clk_hw *hw;
+	u32 val, rate;
+	int i, ret;
+
+	map = syscon_node_to_regmap(dev->of_node);
+	if (IS_ERR(map)) {
+		dev_err(dev, "no syscon regmap\n");
+		return PTR_ERR(map);
+	}
+
+	ar = devm_kzalloc(dev, sizeof(*ar), GFP_KERNEL);
+	if (!ar)
+		return -ENOMEM;
+
+	ar->map = map;
+
+	ar->rcdev.owner = THIS_MODULE;
+	ar->rcdev.nr_resets = 64;
+	ar->rcdev.ops = &aspeed_g6_reset_ops;
+	ar->rcdev.of_node = dev->of_node;
+
+	ret = devm_reset_controller_register(dev, &ar->rcdev);
+	if (ret) {
+		dev_err(dev, "could not register reset controller\n");
+		return ret;
+	}
+
+	/* UART clock div13 setting */
+	regmap_read(map, ASPEED_G6_MISC_CTRL, &val);
+	if (val & UART_DIV13_EN)
+		rate = 24000000 / 13;
+	else
+		rate = 24000000;
+	hw = clk_hw_register_fixed_rate(dev, "uart", NULL, 0, rate);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_UART] = hw;
+
+	/* UART6~13 clock div13 setting */
+	regmap_read(map, 0x80, &val);
+	if (val & BIT(31))
+		rate = 24000000 / 13;
+	else
+		rate = 24000000;
+	hw = clk_hw_register_fixed_rate(dev, "uartx", NULL, 0, rate);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_UARTX] = hw;
+
+	/* EMMC ext clock divider */
+	hw = clk_hw_register_gate(dev, "emmc_extclk_gate", "hpll", 0,
+			scu_g6_base + ASPEED_G6_CLK_SELECTION1, 15, 0,
+			&aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	hw = clk_hw_register_divider_table(dev, "emmc_extclk", "emmc_extclk_gate", 0,
+			scu_g6_base + ASPEED_G6_CLK_SELECTION1, 12, 3, 0,
+			ast2600_div_table,
+			&aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_EMMC] = hw;
+
+	/* SD/SDIO clock divider and gate */
+	hw = clk_hw_register_gate(dev, "sd_extclk_gate", "hpll", 0,
+			scu_g6_base + ASPEED_G6_CLK_SELECTION4, 31, 0,
+			&aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	hw = clk_hw_register_divider_table(dev, "sd_extclk", "sd_extclk_gate",
+			0, scu_g6_base + ASPEED_G6_CLK_SELECTION4, 28, 3, 0,
+			ast2600_div_table,
+			&aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_SDIO] = hw;
+
+	/* MAC1/2 AHB bus clock divider */
+	hw = clk_hw_register_divider_table(dev, "mac12", "hpll", 0,
+			scu_g6_base + ASPEED_G6_CLK_SELECTION1, 16, 3, 0,
+			ast2600_mac_div_table,
+			&aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_MAC12] = hw;
+
+	/* MAC3/4 AHB bus clock divider */
+	hw = clk_hw_register_divider_table(dev, "mac34", "hpll", 0,
+			scu_g6_base + 0x310, 24, 3, 0,
+			ast2600_mac_div_table,
+			&aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_MAC34] = hw;
+
+	/* LPC Host (LHCLK) clock divider */
+	hw = clk_hw_register_divider_table(dev, "lhclk", "hpll", 0,
+			scu_g6_base + ASPEED_G6_CLK_SELECTION1, 20, 3, 0,
+			ast2600_div_table,
+			&aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_LHCLK] = hw;
+
+	/* gfx d1clk : use dp clk */
+	regmap_update_bits(map, ASPEED_G6_CLK_SELECTION1, GENMASK(10, 8), BIT(10));
+	/* SoC Display clock selection */
+	hw = clk_hw_register_mux(dev, "d1clk", d1clk_parent_names,
+			ARRAY_SIZE(d1clk_parent_names), 0,
+			scu_g6_base + ASPEED_G6_CLK_SELECTION1, 8, 3, 0,
+			&aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_D1CLK] = hw;
+
+	//d1 clk div 0x308[17:15] x [14:12] - 8,7,6,5,4,3,2,1
+	regmap_write(map, 0x308, 0x12000); //3x3 = 9
+
+	/* P-Bus (BCLK) clock divider */
+	hw = clk_hw_register_divider_table(dev, "bclk", "hpll", 0,
+			scu_g6_base + ASPEED_G6_CLK_SELECTION1, 20, 3, 0,
+			ast2600_div_table,
+			&aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_BCLK] = hw;
+
+	/* Video Capture clock selection */
+	hw = clk_hw_register_mux(dev, "vclk", vclk_parent_names,
+			ARRAY_SIZE(vclk_parent_names), 0,
+			scu_g6_base + ASPEED_G6_CLK_SELECTION2, 12, 3, 0,
+			&aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_VCLK] = hw;
+
+	/* Video Engine clock divider */
+	hw = clk_hw_register_divider_table(dev, "eclk", NULL, 0,
+			scu_g6_base + ASPEED_G6_CLK_SELECTION1, 28, 3, 0,
+			ast2600_eclk_div_table,
+			&aspeed_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_ECLK] = hw;
+
+	for (i = 0; i < ARRAY_SIZE(aspeed_g6_gates); i++) {
+		const struct aspeed_gate_data *gd = &aspeed_g6_gates[i];
+		u32 gate_flags;
+
+		/* Special case: the USB port 1 clock (bit 14) is always
+		 * working the opposite way from the other ones.
+		 */
+		gate_flags = (gd->clock_idx == 14) ? 0 : CLK_GATE_SET_TO_DISABLE;
+		hw = aspeed_g6_clk_hw_register_gate(dev,
+				gd->name,
+				gd->parent_name,
+				gd->flags,
+				map,
+				gd->clock_idx,
+				gd->reset_idx,
+				gate_flags,
+				&aspeed_clk_lock);
+		if (IS_ERR(hw))
+			return PTR_ERR(hw);
+		aspeed_g6_clk_data->hws[i] = hw;
+	}
+
+	return 0;
+};
+
+static const struct of_device_id aspeed_g6_clk_dt_ids[] = {
+	{ .compatible = "aspeed,ast2600-scu" },
+	{ }
+};
+
+static struct platform_driver aspeed_g6_clk_driver = {
+	.probe  = aspeed_g6_clk_probe,
+	.driver = {
+		.name = "ast2600-clk",
+		.of_match_table = aspeed_g6_clk_dt_ids,
+		.suppress_bind_attrs = true,
+	},
+};
+builtin_platform_driver(aspeed_g6_clk_driver);
+
+static u32 ast2600_a0_axi_ahb_div_table[] = {
+	2, 2, 3, 5,
+};
+
+static u32 ast2600_a1_axi_ahb_div_table[] = {
+	4, 6, 2, 4,
+};
+
+static void __init aspeed_g6_cc(struct regmap *map)
+{
+	struct clk_hw *hw;
+	u32 val, div, chip_id, axi_div, ahb_div;
+
+	clk_hw_register_fixed_rate(NULL, "clkin", NULL, 0, 25000000);
+
+	/*
+	 * High-speed PLL clock derived from the crystal. This the CPU clock,
+	 * and we assume that it is enabled
+	 */
+	regmap_read(map, ASPEED_HPLL_PARAM, &val);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_HPLL] = ast2600_calc_pll("hpll", val);
+
+	regmap_read(map, ASPEED_MPLL_PARAM, &val);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_MPLL] = ast2600_calc_pll("mpll", val);
+
+	regmap_read(map, ASPEED_DPLL_PARAM, &val);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_DPLL] = ast2600_calc_pll("dpll", val);
+
+	regmap_read(map, ASPEED_EPLL_PARAM, &val);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_EPLL] = ast2600_calc_pll("epll", val);
+
+	regmap_read(map, ASPEED_APLL_PARAM, &val);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_APLL] = ast2600_calc_apll("apll", val);
+
+	/* Strap bits 12:11 define the AXI/AHB clock frequency ratio (aka HCLK)*/
+	regmap_read(map, ASPEED_G6_STRAP1, &val);
+	if (val & BIT(16))
+		axi_div = 1;
+	else
+		axi_div = 2;
+
+	regmap_read(map, ASPEED_G6_SILICON_REV, &chip_id);
+	if (chip_id & BIT(16))
+		ahb_div = ast2600_a1_axi_ahb_div_table[(val >> 11) & 0x3];
+	else
+		ahb_div = ast2600_a0_axi_ahb_div_table[(val >> 11) & 0x3];
+
+	hw = clk_hw_register_fixed_factor(NULL, "ahb", "hpll", 0, 1, axi_div * ahb_div);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_AHB] = hw;
+
+	regmap_read(map, ASPEED_G6_CLK_SELECTION1, &val);
+	val = (val >> 23) & 0x7;
+	div = 4 * (val + 1);
+	hw = clk_hw_register_fixed_factor(NULL, "apb1", "hpll", 0, 1, div);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_APB1] = hw;
+
+	regmap_read(map, ASPEED_G6_CLK_SELECTION4, &val);
+	val = (val >> 9) & 0x7;
+	div = 2 * (val + 1);
+	hw = clk_hw_register_fixed_factor(NULL, "apb2", "ahb", 0, 1, div);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_APB2] = hw;
+
+	/* USB 2.0 port1 phy 40MHz clock */
+	hw = clk_hw_register_fixed_rate(NULL, "usb-phy-40m", NULL, 0, 40000000);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_USBPHY_40M] = hw;
+};
+
+static void __init aspeed_g6_cc_init(struct device_node *np)
+{
+	struct regmap *map;
+	int ret;
+	int i;
+
+	scu_g6_base = of_iomap(np, 0);
+	if (!scu_g6_base)
+		return;
+
+	aspeed_g6_clk_data = kzalloc(struct_size(aspeed_g6_clk_data, hws,
+				      ASPEED_G6_NUM_CLKS), GFP_KERNEL);
+	if (!aspeed_g6_clk_data)
+		return;
+
+	/*
+	 * This way all clocks fetched before the platform device probes,
+	 * except those we assign here for early use, will be deferred.
+	 */
+	for (i = 0; i < ASPEED_G6_NUM_CLKS; i++)
+		aspeed_g6_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER);
+
+	/*
+	 * We check that the regmap works on this very first access,
+	 * but as this is an MMIO-backed regmap, subsequent regmap
+	 * access is not going to fail and we skip error checks from
+	 * this point.
+	 */
+	map = syscon_node_to_regmap(np);
+	if (IS_ERR(map)) {
+		pr_err("no syscon regmap\n");
+		return;
+	}
+
+	aspeed_g6_cc(map);
+	aspeed_g6_clk_data->num = ASPEED_G6_NUM_CLKS;
+	ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, aspeed_g6_clk_data);
+	if (ret)
+		pr_err("failed to add DT provider: %d\n", ret);
+};
+CLK_OF_DECLARE_DRIVER(aspeed_cc_g6, "aspeed,ast2600-scu", aspeed_g6_cc_init);
diff --git a/include/dt-bindings/clock/ast2600-clock.h b/include/dt-bindings/clock/ast2600-clock.h
new file mode 100644
index 000000000000..66567bd48d5b
--- /dev/null
+++ b/include/dt-bindings/clock/ast2600-clock.h
@@ -0,0 +1,116 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+#ifndef DT_BINDINGS_AST2600_CLOCK_H
+#define DT_BINDINGS_AST2600_CLOCK_H
+
+#define ASPEED_CLK_GATE_ECLK		0
+#define ASPEED_CLK_GATE_GCLK		1
+
+#define ASPEED_CLK_GATE_MCLK		2
+
+#define ASPEED_CLK_GATE_VCLK		3
+#define ASPEED_CLK_GATE_BCLK		4
+#define ASPEED_CLK_GATE_DCLK		5
+
+#define ASPEED_CLK_GATE_LCLK		6
+#define ASPEED_CLK_GATE_LHCCLK		7
+
+#define ASPEED_CLK_GATE_D1CLK		8
+#define ASPEED_CLK_GATE_YCLK		9
+
+#define ASPEED_CLK_GATE_REF0CLK		10
+#define ASPEED_CLK_GATE_REF1CLK		11
+
+#define ASPEED_CLK_GATE_ESPICLK		12
+
+#define ASPEED_CLK_GATE_USBUHCICLK	13
+#define ASPEED_CLK_GATE_USBPORT1CLK	14
+#define ASPEED_CLK_GATE_USBPORT2CLK	15
+
+#define ASPEED_CLK_GATE_RSACLK		16
+#define ASPEED_CLK_GATE_RVASCLK		17
+
+#define ASPEED_CLK_GATE_MAC1CLK		18
+#define ASPEED_CLK_GATE_MAC2CLK		19
+#define ASPEED_CLK_GATE_MAC3CLK		20
+#define ASPEED_CLK_GATE_MAC4CLK		21
+
+#define ASPEED_CLK_GATE_UART1CLK	22
+#define ASPEED_CLK_GATE_UART2CLK	23
+#define ASPEED_CLK_GATE_UART3CLK	24
+#define ASPEED_CLK_GATE_UART4CLK	25
+#define ASPEED_CLK_GATE_UART5CLK	26
+#define ASPEED_CLK_GATE_UART6CLK	27
+#define ASPEED_CLK_GATE_UART7CLK	28
+#define ASPEED_CLK_GATE_UART8CLK	29
+#define ASPEED_CLK_GATE_UART9CLK	30
+#define ASPEED_CLK_GATE_UART10CLK	31
+#define ASPEED_CLK_GATE_UART11CLK	32
+#define ASPEED_CLK_GATE_UART12CLK	33
+#define ASPEED_CLK_GATE_UART13CLK	34
+
+#define ASPEED_CLK_GATE_SDCLK		35
+#define ASPEED_CLK_GATE_SDEXTCLK	36
+#define ASPEED_CLK_GATE_EMMCCLK		37
+#define ASPEED_CLK_GATE_EMMCEXTCLK	38
+
+#define ASPEED_CLK_GATE_I3C0CLK		39
+#define ASPEED_CLK_GATE_I3C1CLK		40
+#define ASPEED_CLK_GATE_I3C2CLK		41
+#define ASPEED_CLK_GATE_I3C3CLK		42
+#define ASPEED_CLK_GATE_I3C4CLK		43
+#define ASPEED_CLK_GATE_I3C5CLK		44
+#define ASPEED_CLK_GATE_I3C6CLK		45
+#define ASPEED_CLK_GATE_I3C7CLK		46
+
+#define ASPEED_CLK_GATE_FSICLK		47
+
+#define ASPEED_CLK_HPLL			48
+#define ASPEED_CLK_MPLL			49
+#define ASPEED_CLK_DPLL			50
+#define ASPEED_CLK_EPLL			51
+#define ASPEED_CLK_APLL			52
+#define ASPEED_CLK_AHB			53
+#define ASPEED_CLK_APB1			54
+#define ASPEED_CLK_APB2			55
+#define ASPEED_CLK_BCLK			56
+#define ASPEED_CLK_D1CLK		57
+#define ASPEED_CLK_VCLK			58
+#define ASPEED_CLK_LHCLK		59
+#define ASPEED_CLK_UART			60
+#define ASPEED_CLK_UARTX		61
+#define ASPEED_CLK_SDIO			62
+#define ASPEED_CLK_EMMC			63
+#define ASPEED_CLK_ECLK			64
+#define ASPEED_CLK_ECLK_MUX		65
+#define ASPEED_CLK_MAC12		66
+#define ASPEED_CLK_MAC34		67
+#define ASPEED_CLK_USBPHY_40M		68
+
+/* Only list resets here that are not part of a gate */
+#define ASPEED_RESET_SD			56
+#define ASPEED_RESET_ADC		55
+#define ASPEED_RESET_JTAG_MASTER2	54
+#define ASPEED_RESET_I3C_DMA		39
+#define ASPEED_RESET_PWM		37
+#define ASPEED_RESET_PECI		36
+#define ASPEED_RESET_MII		35
+#define ASPEED_RESET_I2C		34
+#define ASPEED_RESET_H2X		31
+#define ASPEED_RESET_GP_MCU		30
+#define ASPEED_RESET_DP_MCU		29
+#define ASPEED_RESET_DP			28
+#define ASPEED_RESET_RC_XDMA		27
+#define ASPEED_RESET_GRAPHICS		26
+#define ASPEED_RESET_DEV_XDMA		25
+#define ASPEED_RESET_DEV_MCTP		24
+#define ASPEED_RESET_RC_MCTP		23
+#define ASPEED_RESET_JTAG_MASTER	22
+#define ASPEED_RESET_PCIE_DEV_O		21
+#define ASPEED_RESET_PCIE_DEV_OEN	20
+#define ASPEED_RESET_PCIE_RC_O		19
+#define ASPEED_RESET_PCIE_RC_OEN	18
+#define ASPEED_RESET_PCI_DP		5
+#define ASPEED_RESET_AHB		1
+#define ASPEED_RESET_SDRAM		0
+
+#endif
-- 
2.23.0.rc1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 1/2] clk: aspeed: Move structures to header
From: Joel Stanley @ 2019-08-16 15:58 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette
  Cc: Ryan Chen, linux-aspeed, Andrew Jeffery, linux-kernel,
	Rob Herring, linux-clk, linux-arm-kernel
In-Reply-To: <20190816155806.22869-1-joel@jms.id.au>

They will be reused by the ast2600 driver.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/clk/clk-aspeed.c | 63 ++--------------------------------
 drivers/clk/clk-aspeed.h | 74 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 61 deletions(-)
 create mode 100644 drivers/clk/clk-aspeed.h

diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 898291501f45..44df54d87ad4 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -14,6 +14,8 @@
 
 #include <dt-bindings/clock/aspeed-clock.h>
 
+#include "clk-aspeed.h"
+
 #define ASPEED_NUM_CLKS		36
 
 #define ASPEED_RESET2_OFFSET	32
@@ -42,48 +44,6 @@ static struct clk_hw_onecell_data *aspeed_clk_data;
 
 static void __iomem *scu_base;
 
-/**
- * struct aspeed_gate_data - Aspeed gated clocks
- * @clock_idx: bit used to gate this clock in the clock register
- * @reset_idx: bit used to reset this IP in the reset register. -1 if no
- *             reset is required when enabling the clock
- * @name: the clock name
- * @parent_name: the name of the parent clock
- * @flags: standard clock framework flags
- */
-struct aspeed_gate_data {
-	u8		clock_idx;
-	s8		reset_idx;
-	const char	*name;
-	const char	*parent_name;
-	unsigned long	flags;
-};
-
-/**
- * struct aspeed_clk_gate - Aspeed specific clk_gate structure
- * @hw:		handle between common and hardware-specific interfaces
- * @reg:	register controlling gate
- * @clock_idx:	bit used to gate this clock in the clock register
- * @reset_idx:	bit used to reset this IP in the reset register. -1 if no
- *		reset is required when enabling the clock
- * @flags:	hardware-specific flags
- * @lock:	register lock
- *
- * Some of the clocks in the Aspeed SoC must be put in reset before enabling.
- * This modified version of clk_gate allows an optional reset bit to be
- * specified.
- */
-struct aspeed_clk_gate {
-	struct clk_hw	hw;
-	struct regmap	*map;
-	u8		clock_idx;
-	s8		reset_idx;
-	u8		flags;
-	spinlock_t	*lock;
-};
-
-#define to_aspeed_clk_gate(_hw) container_of(_hw, struct aspeed_clk_gate, hw)
-
 /* TODO: ask Aspeed about the actual parent data */
 static const struct aspeed_gate_data aspeed_gates[] = {
 	/*				 clk rst   name			parent	flags */
@@ -208,13 +168,6 @@ static struct clk_hw *aspeed_ast2500_calc_pll(const char *name, u32 val)
 			mult, div);
 }
 
-struct aspeed_clk_soc_data {
-	const struct clk_div_table *div_table;
-	const struct clk_div_table *eclk_div_table;
-	const struct clk_div_table *mac_div_table;
-	struct clk_hw *(*calc_pll)(const char *name, u32 val);
-};
-
 static const struct aspeed_clk_soc_data ast2500_data = {
 	.div_table = ast2500_div_table,
 	.eclk_div_table = ast2500_eclk_div_table,
@@ -315,18 +268,6 @@ static const struct clk_ops aspeed_clk_gate_ops = {
 	.is_enabled = aspeed_clk_is_enabled,
 };
 
-/**
- * struct aspeed_reset - Aspeed reset controller
- * @map: regmap to access the containing system controller
- * @rcdev: reset controller device
- */
-struct aspeed_reset {
-	struct regmap			*map;
-	struct reset_controller_dev	rcdev;
-};
-
-#define to_aspeed_reset(p) container_of((p), struct aspeed_reset, rcdev)
-
 static const u8 aspeed_resets[] = {
 	/* SCU04 resets */
 	[ASPEED_RESET_XDMA]	= 25,
diff --git a/drivers/clk/clk-aspeed.h b/drivers/clk/clk-aspeed.h
new file mode 100644
index 000000000000..92d384367c25
--- /dev/null
+++ b/drivers/clk/clk-aspeed.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Structures used by ASPEED clock drivers
+ *
+ * Copyright 2019 IBM Corp.
+ */
+
+/**
+ * struct aspeed_gate_data - Aspeed gated clocks
+ * @clock_idx: bit used to gate this clock in the clock register
+ * @reset_idx: bit used to reset this IP in the reset register. -1 if no
+ *             reset is required when enabling the clock
+ * @name: the clock name
+ * @parent_name: the name of the parent clock
+ * @flags: standard clock framework flags
+ */
+struct aspeed_gate_data {
+	u8		clock_idx;
+	s8		reset_idx;
+	const char	*name;
+	const char	*parent_name;
+	unsigned long	flags;
+};
+
+/**
+ * struct aspeed_clk_gate - Aspeed specific clk_gate structure
+ * @hw:		handle between common and hardware-specific interfaces
+ * @reg:	register controlling gate
+ * @clock_idx:	bit used to gate this clock in the clock register
+ * @reset_idx:	bit used to reset this IP in the reset register. -1 if no
+ *		reset is required when enabling the clock
+ * @flags:	hardware-specific flags
+ * @lock:	register lock
+ *
+ * Some of the clocks in the Aspeed SoC must be put in reset before enabling.
+ * This modified version of clk_gate allows an optional reset bit to be
+ * specified.
+ */
+struct aspeed_clk_gate {
+	struct clk_hw	hw;
+	struct regmap	*map;
+	u8		clock_idx;
+	s8		reset_idx;
+	u8		flags;
+	spinlock_t	*lock;
+};
+
+#define to_aspeed_clk_gate(_hw) container_of(_hw, struct aspeed_clk_gate, hw)
+
+/**
+ * struct aspeed_reset - Aspeed reset controller
+ * @map: regmap to access the containing system controller
+ * @rcdev: reset controller device
+ */
+struct aspeed_reset {
+	struct regmap			*map;
+	struct reset_controller_dev	rcdev;
+};
+
+#define to_aspeed_reset(p) container_of((p), struct aspeed_reset, rcdev)
+
+/**
+ * struct aspeed_clk_soc_data - Aspeed SoC specific divisor information
+ * @div_table: Common divider lookup table
+ * @eclk_div_table: Divider lookup table for ECLK
+ * @mac_div_table: Divider lookup table for MAC (Ethernet) clocks
+ * @calc_pll: Callback to maculate common PLL settings
+ */
+struct aspeed_clk_soc_data {
+	const struct clk_div_table *div_table;
+	const struct clk_div_table *eclk_div_table;
+	const struct clk_div_table *mac_div_table;
+	struct clk_hw *(*calc_pll)(const char *name, u32 val);
+};
-- 
2.23.0.rc1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 0/2] clk: Add driver for ast2600
From: Joel Stanley @ 2019-08-16 15:58 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette
  Cc: Ryan Chen, linux-aspeed, Andrew Jeffery, linux-kernel,
	Rob Herring, linux-clk, linux-arm-kernel

Hello clock maintainers,

This adds a new driver for the ast2600 BMC's clocks. It's a separate
from the existing aspeed one as the ast2600 changes enough from the
previous generation to make it hard to support with one driver.

It has been tested on the ast2600 evaluation board.

Joel Stanley (2):
  clk: aspeed: Move structures to header
  clk: Add support for AST2600 SoC

 drivers/clk/Makefile                      |   1 +
 drivers/clk/clk-aspeed.c                  |  63 +-
 drivers/clk/clk-aspeed.h                  |  76 +++
 drivers/clk/clk-ast2600.c                 | 711 ++++++++++++++++++++++
 include/dt-bindings/clock/ast2600-clock.h | 116 ++++
 5 files changed, 906 insertions(+), 61 deletions(-)
 create mode 100644 drivers/clk/clk-aspeed.h
 create mode 100644 drivers/clk/clk-ast2600.c
 create mode 100644 include/dt-bindings/clock/ast2600-clock.h

-- 
2.23.0.rc1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v5 3/3] coresight: etm4x: save/restore state for external agents
From: Andrew Murray @ 2019-08-16 15:46 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Alexander Shishkin
  Cc: Al.Grant, coresight, Leo Yan, Sudeep Holla, linux-arm-kernel,
	Mike Leach
In-Reply-To: <20190816154615.39854-1-andrew.murray@arm.com>

Some hardware will ignore bit TRCPDCR.PU which is used to signal
to hardware that power should not be removed from the trace unit. Much like
self-hosted debug, we should also save/restore the trace unit state when
it is in use by external agents.

We wish to avoid saving the hardware state when coresight isn't in use
to reduce PM latency - However as external trace/debug is designed to be
unintrusive to the CPU, the only way of determining that an external agent is
present is to read the claim tags (TRCCLAIMCLR). Unfortunately this register
needs power and clocking - something it won't have when coresight isn't in use.
We also don't want to temporarily enable it due to the latency and PM context.

Let's compromise by adding a module parameter that will keep the trace unit
powered and clocked, thus allowing us to only save/restore state when external
trace (or self-hosted) is in use. Though please note that this doesn't allow
for tracing from boot on hardware that needs save/restore as the CPU may idle
prior to the ETMv4 driver starting and adding PM hooks to save/restore.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x.c | 27 ++++++++++++++++---
 drivers/hwtracing/coresight/coresight.c       |  2 +-
 include/linux/coresight.h                     |  7 +++++
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 35a524eec36d..c5d527f7cbd5 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -42,11 +42,12 @@ MODULE_PARM_DESC(boot_enable, "Enable tracing on boot");
 #define PARAM_PM_SAVE_FIRMWARE	  0 /* save self-hosted state as per firmware */
 #define PARAM_PM_SAVE_NEVER	  1 /* never save any state */
 #define PARAM_PM_SAVE_SELF_HOSTED 2 /* save self-hosted state only */
+#define PARAM_PM_SAVE_EXTERNAL	  3 /* save all state (keeps power on) */
 
 static int pm_save_enable = PARAM_PM_SAVE_FIRMWARE;
 module_param(pm_save_enable, int, 0444);
 MODULE_PARM_DESC(pm_save_enable,
-	"Save/restore state on power down: 1 = never, 2 = self-hosted");
+	"Save/restore state on power down: 1 = never, 2 = self-hosted, 3 = self-hosted/external");
 
 /* The number of ETMv4 currently registered */
 static int etm4_count;
@@ -1331,6 +1332,22 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 	CS_LOCK(drvdata->base);
 }
 
+static bool etm4_coresight_in_use(struct etmv4_drvdata *drvdata)
+{
+	/* Self-hosted session in progress? */
+	if (local_read(&drvdata->mode))
+		return true;
+
+	/* External agents can be detected through claim tags however we
+	 * only read these tags if the trace unit is powered.
+	 */
+	if (drvdata->csdev && pm_runtime_active(drvdata->csdev->dev.parent))
+		if (coresight_is_claimed_any(drvdata->base))
+			return true;
+
+	return false;
+}
+
 static int etm4_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
 			      void *v)
 {
@@ -1350,8 +1367,8 @@ static int etm4_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
 
 	switch (cmd) {
 	case CPU_PM_ENTER:
-		/* save the state if self-hosted coresight is in use */
-		if (local_read(&drvdata->mode))
+		/* Save the state if coresight is in use */
+		if (etm4_coresight_in_use(drvdata))
 			if (etm4_cpu_save(drvdata))
 				return NOTIFY_BAD;
 		break;
@@ -1488,7 +1505,9 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
 		goto err_arch_supported;
 	}
 
-	pm_runtime_put(&adev->dev);
+	if (pm_save_enable != PARAM_PM_SAVE_EXTERNAL)
+		pm_runtime_put(&adev->dev);
+
 	dev_info(&drvdata->csdev->dev, "CPU%d: ETM v%d.%d initialized\n",
 		 drvdata->cpu, drvdata->arch >> 4, drvdata->arch & 0xf);
 
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
index e6ca899fea4e..474b7372864e 100644
--- a/drivers/hwtracing/coresight/coresight.c
+++ b/drivers/hwtracing/coresight/coresight.c
@@ -140,7 +140,7 @@ static inline bool coresight_is_claimed_self_hosted(void __iomem *base)
 	return coresight_read_claim_tags(base) == CORESIGHT_CLAIM_SELF_HOSTED;
 }
 
-static inline bool coresight_is_claimed_any(void __iomem *base)
+bool coresight_is_claimed_any(void __iomem *base)
 {
 	return coresight_read_claim_tags(base) != 0;
 }
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 44e552de419c..65bfd2cb0283 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -286,6 +286,8 @@ extern void coresight_disclaim_device_unlocked(void __iomem *base);
 extern char *coresight_alloc_device_name(struct coresight_dev_list *devs,
 					 struct device *dev);
 
+extern bool coresight_is_claimed_any(void __iomem *base);
+
 extern bool coresight_loses_context_with_cpu(struct device *dev);
 #else
 static inline struct coresight_device *
@@ -309,6 +311,11 @@ static inline int coresight_claim_device(void __iomem *base)
 static inline void coresight_disclaim_device(void __iomem *base) {}
 static inline void coresight_disclaim_device_unlocked(void __iomem *base) {}
 
+static inline bool coresight_is_claimed_any(void __iomem *base)
+{
+	return false;
+}
+
 static inline bool coresight_loses_context_with_cpu(struct device *dev)
 {
 	return false;
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v5 2/3] dt-bindings: arm: coresight: Add support for coresight-loses-context-with-cpu
From: Andrew Murray @ 2019-08-16 15:46 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Alexander Shishkin
  Cc: Al.Grant, coresight, Leo Yan, Sudeep Holla, linux-arm-kernel,
	Mike Leach
In-Reply-To: <20190816154615.39854-1-andrew.murray@arm.com>

Some coresight components, because of choices made during hardware
integration, require their state to be saved and restored across CPU low
power states.

The software has no reliable method of detecting when save/restore is
required thus let's add a binding to inform the kernel.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 Documentation/devicetree/bindings/arm/coresight.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
index fcc3bacfd8bc..d02c42d21f2f 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -87,6 +87,15 @@ its hardware characteristcs.
 
 	* port or ports: see "Graph bindings for Coresight" below.
 
+* Optional properties for all components:
+
+	* arm,coresight-loses-context-with-cpu : boolean. Indicates that the
+	  hardware will lose register context on CPU power down (e.g. CPUIdle).
+	  An example of where this may be needed are systems which contain a
+	  coresight component and CPU in the same power domain. When the CPU
+	  powers down the coresight component also powers down and loses its
+	  context. This property is currently only used for the ETM 4.x driver.
+
 * Optional properties for ETM/PTMs:
 
 	* arm,cp14: must be present if the system accesses ETM/PTM management
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v5 1/3] coresight: etm4x: save/restore state across CPU low power states
From: Andrew Murray @ 2019-08-16 15:46 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Alexander Shishkin
  Cc: Al.Grant, coresight, Leo Yan, Sudeep Holla, linux-arm-kernel,
	Mike Leach
In-Reply-To: <20190816154615.39854-1-andrew.murray@arm.com>

Some hardware will ignore bit TRCPDCR.PU which is used to signal
to hardware that power should not be removed from the trace unit.
Let's mitigate against this by conditionally saving and restoring
the trace unit state when the CPU enters low power states.

This patchset introduces a firmware property named
'arm,coresight-loses-context-with-cpu' - when this is present the
hardware state will be conditionally saved and restored.

A module parameter 'pm_save_enable' is also introduced which can
be configured to override the firmware property. This can be set
to never allow save/restore or to conditionally allow it (only for
self-hosted). The default value is determined by firmware.

We avoid saving the hardware state when self-hosted coresight isn't
in use to reduce PM latency - we can't determine this by reading the
claim tags (TRCCLAIMCLR) as these are 'trace' registers which need
power and clocking, something we can't easily provide in the PM
context. Therefore we rely on the existing drvdata->mode internal
state that is set when self-hosted coresight is used (and powered).

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x.c | 318 ++++++++++++++++++
 drivers/hwtracing/coresight/coresight-etm4x.h |  64 ++++
 drivers/hwtracing/coresight/coresight.c       |   6 +
 include/linux/coresight.h                     |   6 +
 4 files changed, 394 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index a128b5063f46..35a524eec36d 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -18,6 +18,7 @@
 #include <linux/stat.h>
 #include <linux/clk.h>
 #include <linux/cpu.h>
+#include <linux/cpu_pm.h>
 #include <linux/coresight.h>
 #include <linux/coresight-pmu.h>
 #include <linux/pm_wakeup.h>
@@ -26,6 +27,7 @@
 #include <linux/uaccess.h>
 #include <linux/perf_event.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <asm/sections.h>
 #include <asm/local.h>
 #include <asm/virt.h>
@@ -37,6 +39,15 @@ static int boot_enable;
 module_param(boot_enable, int, 0444);
 MODULE_PARM_DESC(boot_enable, "Enable tracing on boot");
 
+#define PARAM_PM_SAVE_FIRMWARE	  0 /* save self-hosted state as per firmware */
+#define PARAM_PM_SAVE_NEVER	  1 /* never save any state */
+#define PARAM_PM_SAVE_SELF_HOSTED 2 /* save self-hosted state only */
+
+static int pm_save_enable = PARAM_PM_SAVE_FIRMWARE;
+module_param(pm_save_enable, int, 0444);
+MODULE_PARM_DESC(pm_save_enable,
+	"Save/restore state on power down: 1 = never, 2 = self-hosted");
+
 /* The number of ETMv4 currently registered */
 static int etm4_count;
 static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
@@ -54,6 +65,14 @@ static void etm4_os_unlock(struct etmv4_drvdata *drvdata)
 	isb();
 }
 
+static void etm4_os_lock(struct etmv4_drvdata *drvdata)
+{
+	/* Writing 0x1 to TRCOSLAR locks the trace registers */
+	writel_relaxed(0x1, drvdata->base + TRCOSLAR);
+	drvdata->os_unlock = false;
+	isb();
+}
+
 static bool etm4_arch_supported(u8 arch)
 {
 	/* Mask out the minor version number */
@@ -1085,6 +1104,288 @@ static void etm4_init_trace_id(struct etmv4_drvdata *drvdata)
 	drvdata->trcid = coresight_get_trace_id(drvdata->cpu);
 }
 
+#ifdef CONFIG_CPU_PM
+static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
+{
+	int i, ret = 0;
+	struct etmv4_save_state *state;
+	struct device *etm_dev = &drvdata->csdev->dev;
+
+	/*
+	 * As recommended by 3.4.1 ("The procedure when powering down the PE")
+	 * of ARM IHI 0064D
+	 */
+	dsb(sy);
+	isb();
+
+	CS_UNLOCK(drvdata->base);
+
+	/* Lock the OS lock to disable trace and external debugger access */
+	etm4_os_lock(drvdata);
+
+	/* wait for TRCSTATR.PMSTABLE to go up */
+	if (coresight_timeout(drvdata->base, TRCSTATR,
+					TRCSTATR_PMSTABLE_BIT, 1)) {
+		dev_err(etm_dev,
+			"timeout while waiting for PM Stable Status\n");
+		etm4_os_unlock(drvdata);
+		ret = -EBUSY;
+		goto out;
+	}
+
+	state = drvdata->save_state;
+
+	state->trcprgctlr = readl(drvdata->base + TRCPRGCTLR);
+	state->trcprocselr = readl(drvdata->base + TRCPROCSELR);
+	state->trcconfigr = readl(drvdata->base + TRCCONFIGR);
+	state->trcauxctlr = readl(drvdata->base + TRCAUXCTLR);
+	state->trceventctl0r = readl(drvdata->base + TRCEVENTCTL0R);
+	state->trceventctl1r = readl(drvdata->base + TRCEVENTCTL1R);
+	state->trcstallctlr = readl(drvdata->base + TRCSTALLCTLR);
+	state->trctsctlr = readl(drvdata->base + TRCTSCTLR);
+	state->trcsyncpr = readl(drvdata->base + TRCSYNCPR);
+	state->trcccctlr = readl(drvdata->base + TRCCCCTLR);
+	state->trcbbctlr = readl(drvdata->base + TRCBBCTLR);
+	state->trctraceidr = readl(drvdata->base + TRCTRACEIDR);
+	state->trcqctlr = readl(drvdata->base + TRCQCTLR);
+
+	state->trcvictlr = readl(drvdata->base + TRCVICTLR);
+	state->trcviiectlr = readl(drvdata->base + TRCVIIECTLR);
+	state->trcvissctlr = readl(drvdata->base + TRCVISSCTLR);
+	state->trcvipcssctlr = readl(drvdata->base + TRCVIPCSSCTLR);
+	state->trcvdctlr = readl(drvdata->base + TRCVDCTLR);
+	state->trcvdsacctlr = readl(drvdata->base + TRCVDSACCTLR);
+	state->trcvdarcctlr = readl(drvdata->base + TRCVDARCCTLR);
+
+	for (i = 0; i < drvdata->nrseqstate; i++)
+		state->trcseqevr[i] = readl(drvdata->base + TRCSEQEVRn(i));
+
+	state->trcseqrstevr = readl(drvdata->base + TRCSEQRSTEVR);
+	state->trcseqstr = readl(drvdata->base + TRCSEQSTR);
+	state->trcextinselr = readl(drvdata->base + TRCEXTINSELR);
+
+	for (i = 0; i < drvdata->nr_cntr; i++) {
+		state->trccntrldvr[i] = readl(drvdata->base + TRCCNTRLDVRn(i));
+		state->trccntctlr[i] = readl(drvdata->base + TRCCNTCTLRn(i));
+		state->trccntvr[i] = readl(drvdata->base + TRCCNTVRn(i));
+	}
+
+	for (i = 0; i < drvdata->nr_resource * 2; i++)
+		state->trcrsctlr[i] = readl(drvdata->base + TRCRSCTLRn(i));
+
+	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
+		state->trcssccr[i] = readl(drvdata->base + TRCSSCCRn(i));
+		state->trcsscsr[i] = readl(drvdata->base + TRCSSCSRn(i));
+		state->trcsspcicr[i] = readl(drvdata->base + TRCSSPCICRn(i));
+	}
+
+	for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) {
+		state->trcacvr[i] = readl(drvdata->base + TRCACVRn(i));
+		state->trcacatr[i] = readl(drvdata->base + TRCACATRn(i));
+	}
+
+	/*
+	 * Data trace stream is architecturally prohibited for A profile cores
+	 * so we don't save (or later restore) trcdvcvr and trcdvcmr - As per
+	 * section 1.3.4 ("Possible functional configurations of an ETMv4 trace
+	 * unit") of ARM IHI 0064D.
+	 */
+
+	for (i = 0; i < drvdata->numcidc; i++)
+		state->trccidcvr[i] = readl(drvdata->base + TRCCIDCVRn(i));
+
+	for (i = 0; i < drvdata->numvmidc; i++)
+		state->trcvmidcvr[i] = readl(drvdata->base + TRCVMIDCVRn(i));
+
+	state->trccidcctlr0 = readl(drvdata->base + TRCCIDCCTLR0);
+	state->trccidcctlr1 = readl(drvdata->base + TRCCIDCCTLR1);
+
+	state->trcvmidcctlr0 = readl(drvdata->base + TRCVMIDCCTLR0);
+	state->trcvmidcctlr0 = readl(drvdata->base + TRCVMIDCCTLR1);
+
+	state->trcclaimset = readl(drvdata->base + TRCCLAIMCLR);
+
+	state->trcpdcr = readl(drvdata->base + TRCPDCR);
+
+	/* wait for TRCSTATR.IDLE to go up */
+	if (coresight_timeout(drvdata->base, TRCSTATR, TRCSTATR_IDLE_BIT, 1)) {
+		dev_err(etm_dev,
+			"timeout while waiting for Idle Trace Status\n");
+		etm4_os_unlock(drvdata);
+		ret = -EBUSY;
+		goto out;
+	}
+
+	drvdata->state_needs_restore = true;
+
+	/*
+	 * Power can be removed from the trace unit now. We do this to
+	 * potentially save power on systems that respect the TRCPDCR_PU
+	 * despite requesting software to save/restore state.
+	 */
+	writel_relaxed((state->trcpdcr & ~TRCPDCR_PU),
+			drvdata->base + TRCPDCR);
+
+out:
+	CS_LOCK(drvdata->base);
+	return ret;
+}
+
+static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
+{
+	int i;
+	struct etmv4_save_state *state = drvdata->save_state;
+
+	CS_UNLOCK(drvdata->base);
+
+	writel_relaxed(state->trcclaimset, drvdata->base + TRCCLAIMSET);
+
+	writel_relaxed(state->trcprgctlr, drvdata->base + TRCPRGCTLR);
+	writel_relaxed(state->trcprocselr, drvdata->base + TRCPROCSELR);
+	writel_relaxed(state->trcconfigr, drvdata->base + TRCCONFIGR);
+	writel_relaxed(state->trcauxctlr, drvdata->base + TRCAUXCTLR);
+	writel_relaxed(state->trceventctl0r, drvdata->base + TRCEVENTCTL0R);
+	writel_relaxed(state->trceventctl1r, drvdata->base + TRCEVENTCTL1R);
+	writel_relaxed(state->trcstallctlr, drvdata->base + TRCSTALLCTLR);
+	writel_relaxed(state->trctsctlr, drvdata->base + TRCTSCTLR);
+	writel_relaxed(state->trcsyncpr, drvdata->base + TRCSYNCPR);
+	writel_relaxed(state->trcccctlr, drvdata->base + TRCCCCTLR);
+	writel_relaxed(state->trcbbctlr, drvdata->base + TRCBBCTLR);
+	writel_relaxed(state->trctraceidr, drvdata->base + TRCTRACEIDR);
+	writel_relaxed(state->trcqctlr, drvdata->base + TRCQCTLR);
+
+	writel_relaxed(state->trcvictlr, drvdata->base + TRCVICTLR);
+	writel_relaxed(state->trcviiectlr, drvdata->base + TRCVIIECTLR);
+	writel_relaxed(state->trcvissctlr, drvdata->base + TRCVISSCTLR);
+	writel_relaxed(state->trcvipcssctlr, drvdata->base + TRCVIPCSSCTLR);
+	writel_relaxed(state->trcvdctlr, drvdata->base + TRCVDCTLR);
+	writel_relaxed(state->trcvdsacctlr, drvdata->base + TRCVDSACCTLR);
+	writel_relaxed(state->trcvdarcctlr, drvdata->base + TRCVDARCCTLR);
+
+	for (i = 0; i < drvdata->nrseqstate; i++)
+		writel_relaxed(state->trcseqevr[i],
+					drvdata->base + TRCSEQEVRn(i));
+
+	writel_relaxed(state->trcseqrstevr, drvdata->base + TRCSEQRSTEVR);
+	writel_relaxed(state->trcseqstr, drvdata->base + TRCSEQSTR);
+	writel_relaxed(state->trcextinselr, drvdata->base + TRCEXTINSELR);
+
+	for (i = 0; i < drvdata->nr_cntr; i++) {
+		writel_relaxed(state->trccntrldvr[i],
+					drvdata->base + TRCCNTRLDVRn(i));
+		writel_relaxed(state->trccntctlr[i],
+					drvdata->base + TRCCNTCTLRn(i));
+		writel_relaxed(state->trccntvr[i],
+					drvdata->base + TRCCNTVRn(i));
+	}
+
+	for (i = 0; i < drvdata->nr_resource * 2; i++)
+		writel_relaxed(state->trcrsctlr[i],
+					drvdata->base + TRCRSCTLRn(i));
+
+	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
+		writel_relaxed(state->trcssccr[i],
+					drvdata->base + TRCSSCCRn(i));
+		writel_relaxed(state->trcsscsr[i],
+					drvdata->base + TRCSSCSRn(i));
+		writel_relaxed(state->trcsspcicr[i],
+					drvdata->base + TRCSSPCICRn(i));
+	}
+
+	for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) {
+		writel_relaxed(state->trcacvr[i],
+					drvdata->base + TRCACVRn(i));
+		writel_relaxed(state->trcacatr[i],
+					drvdata->base + TRCACATRn(i));
+	}
+
+	for (i = 0; i < drvdata->numcidc; i++)
+		writel_relaxed(state->trccidcvr[i],
+					drvdata->base + TRCCIDCVRn(i));
+
+	for (i = 0; i < drvdata->numvmidc; i++)
+		writel_relaxed(state->trcvmidcvr[i],
+					drvdata->base + TRCVMIDCVRn(i));
+
+	writel_relaxed(state->trccidcctlr0, drvdata->base + TRCCIDCCTLR0);
+	writel_relaxed(state->trccidcctlr1, drvdata->base + TRCCIDCCTLR1);
+
+	writel_relaxed(state->trcvmidcctlr0, drvdata->base + TRCVMIDCCTLR0);
+	writel_relaxed(state->trcvmidcctlr0, drvdata->base + TRCVMIDCCTLR1);
+
+	writel_relaxed(state->trcclaimset, drvdata->base + TRCCLAIMSET);
+
+	writel_relaxed(state->trcpdcr, drvdata->base + TRCPDCR);
+
+	drvdata->state_needs_restore = false;
+
+	/*
+	 * As recommended by section 4.3.7 ("Synchronization when using the
+	 * memory-mapped interface") of ARM IHI 0064D
+	 */
+	dsb(sy);
+	isb();
+
+	/* Unlock the OS lock to re-enable trace and external debug access */
+	etm4_os_unlock(drvdata);
+	CS_LOCK(drvdata->base);
+}
+
+static int etm4_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
+			      void *v)
+{
+	struct etmv4_drvdata *drvdata;
+	unsigned int cpu = smp_processor_id();
+
+	if (!etmdrvdata[cpu])
+		return 0;
+
+	drvdata = etmdrvdata[cpu];
+
+	if (!drvdata->save_state)
+		return NOTIFY_OK;
+
+	if (WARN_ON_ONCE(drvdata->cpu != cpu))
+		return NOTIFY_BAD;
+
+	switch (cmd) {
+	case CPU_PM_ENTER:
+		/* save the state if self-hosted coresight is in use */
+		if (local_read(&drvdata->mode))
+			if (etm4_cpu_save(drvdata))
+				return NOTIFY_BAD;
+		break;
+	case CPU_PM_EXIT:
+	case CPU_PM_ENTER_FAILED:
+		/* trcclaimset is set when there is state to restore */
+		if (drvdata->state_needs_restore)
+			etm4_cpu_restore(drvdata);
+		break;
+	default:
+		return NOTIFY_DONE;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block etm4_cpu_pm_nb = {
+	.notifier_call = etm4_cpu_pm_notify,
+};
+
+static int etm4_cpu_pm_register(void)
+{
+	return cpu_pm_register_notifier(&etm4_cpu_pm_nb);
+}
+
+static void etm4_cpu_pm_unregister(void)
+{
+	cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
+}
+#else
+static int etm4_cpu_pm_register(void) { return 0; }
+static void etm4_cpu_pm_unregister(void) { }
+#endif
+
 static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
 {
 	int ret;
@@ -1101,6 +1402,17 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
 
 	dev_set_drvdata(dev, drvdata);
 
+	if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE)
+		pm_save_enable = coresight_loses_context_with_cpu(dev) ?
+			       PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;
+
+	if (pm_save_enable != PARAM_PM_SAVE_NEVER) {
+		drvdata->save_state = devm_kmalloc(dev,
+				sizeof(struct etmv4_save_state), GFP_KERNEL);
+		if (!drvdata->save_state)
+			return -ENOMEM;
+	}
+
 	/* Validity for the resource is already checked by the AMBA core */
 	base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(base))
@@ -1135,6 +1447,10 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
 		if (ret < 0)
 			goto err_arch_supported;
 		hp_online = ret;
+
+		ret = etm4_cpu_pm_register();
+		if (ret)
+			goto err_arch_supported;
 	}
 
 	cpus_read_unlock();
@@ -1185,6 +1501,8 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
 
 err_arch_supported:
 	if (--etm4_count == 0) {
+		etm4_cpu_pm_unregister();
+
 		cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
 		if (hp_online)
 			cpuhp_remove_state_nocalls(hp_online);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 4523f10ddd0f..546d790cb01b 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -175,6 +175,7 @@
 					 ETM_MODE_EXCL_USER)
 
 #define TRCSTATR_IDLE_BIT		0
+#define TRCSTATR_PMSTABLE_BIT		1
 #define ETM_DEFAULT_ADDR_COMP		0
 
 /* PowerDown Control Register bits */
@@ -281,6 +282,65 @@ struct etmv4_config {
 	u32				ext_inp;
 };
 
+/**
+ * struct etm4_save_state - state to be preserved when ETM is without power
+ */
+struct etmv4_save_state {
+	u32	trcprgctlr;
+	u32	trcprocselr;
+	u32	trcconfigr;
+	u32	trcauxctlr;
+	u32	trceventctl0r;
+	u32	trceventctl1r;
+	u32	trcstallctlr;
+	u32	trctsctlr;
+	u32	trcsyncpr;
+	u32	trcccctlr;
+	u32	trcbbctlr;
+	u32	trctraceidr;
+	u32	trcqctlr;
+
+	u32	trcvictlr;
+	u32	trcviiectlr;
+	u32	trcvissctlr;
+	u32	trcvipcssctlr;
+	u32	trcvdctlr;
+	u32	trcvdsacctlr;
+	u32	trcvdarcctlr;
+
+	u32	trcseqevr[ETM_MAX_SEQ_STATES];
+	u32	trcseqrstevr;
+	u32	trcseqstr;
+	u32	trcextinselr;
+	u32	trccntrldvr[ETMv4_MAX_CNTR];
+	u32	trccntctlr[ETMv4_MAX_CNTR];
+	u32	trccntvr[ETMv4_MAX_CNTR];
+
+	u32	trcrsctlr[ETM_MAX_RES_SEL * 2];
+
+	u32	trcssccr[ETM_MAX_SS_CMP];
+	u32	trcsscsr[ETM_MAX_SS_CMP];
+	u32	trcsspcicr[ETM_MAX_SS_CMP];
+
+	u64	trcacvr[ETM_MAX_SINGLE_ADDR_CMP];
+	u64	trcacatr[ETM_MAX_SINGLE_ADDR_CMP];
+	u64	trccidcvr[ETMv4_MAX_CTXID_CMP];
+	u32	trcvmidcvr[ETM_MAX_VMID_CMP];
+	u32	trccidcctlr0;
+	u32	trccidcctlr1;
+	u32	trcvmidcctlr0;
+	u32	trcvmidcctlr1;
+
+	u32	trcclaimset;
+
+	u32	cntr_val[ETMv4_MAX_CNTR];
+	u32	seq_state;
+	u32	vinst_ctrl;
+	u32	ss_status[ETM_MAX_SS_CMP];
+
+	u32	trcpdcr;
+};
+
 /**
  * struct etm4_drvdata - specifics associated to an ETM component
  * @base:       Memory mapped base address for this component.
@@ -336,6 +396,8 @@ struct etmv4_config {
  * @atbtrig:	If the implementation can support ATB triggers
  * @lpoverride:	If the implementation can support low-power state over.
  * @config:	structure holding configuration parameters.
+ * @save_state:	State to be preserved across power loss
+ * @state_needs_restore: True when there is context to restore after PM exit
  */
 struct etmv4_drvdata {
 	void __iomem			*base;
@@ -381,6 +443,8 @@ struct etmv4_drvdata {
 	bool				atbtrig;
 	bool				lpoverride;
 	struct etmv4_config		config;
+	struct etmv4_save_state		*save_state;
+	bool				state_needs_restore;
 };
 
 /* Address comparator access types */
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
index 6453c67a4d01..e6ca899fea4e 100644
--- a/drivers/hwtracing/coresight/coresight.c
+++ b/drivers/hwtracing/coresight/coresight.c
@@ -1308,6 +1308,12 @@ static inline int coresight_search_device_idx(struct coresight_dev_list *dict,
 	return -ENOENT;
 }
 
+bool coresight_loses_context_with_cpu(struct device *dev)
+{
+	return fwnode_property_present(dev_fwnode(dev),
+				       "arm,coresight-loses-context-with-cpu");
+}
+
 /*
  * coresight_alloc_device_name - Get an index for a given device in the
  * device index list specific to a driver. An index is allocated for a
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index a2b68823717b..44e552de419c 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -285,6 +285,8 @@ extern void coresight_disclaim_device(void __iomem *base);
 extern void coresight_disclaim_device_unlocked(void __iomem *base);
 extern char *coresight_alloc_device_name(struct coresight_dev_list *devs,
 					 struct device *dev);
+
+extern bool coresight_loses_context_with_cpu(struct device *dev);
 #else
 static inline struct coresight_device *
 coresight_register(struct coresight_desc *desc) { return NULL; }
@@ -307,6 +309,10 @@ static inline int coresight_claim_device(void __iomem *base)
 static inline void coresight_disclaim_device(void __iomem *base) {}
 static inline void coresight_disclaim_device_unlocked(void __iomem *base) {}
 
+static inline bool coresight_loses_context_with_cpu(struct device *dev)
+{
+	return false;
+}
 #endif
 
 extern int coresight_get_cpu(struct device *dev);
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v5 0/3] coresight: etm4x: save/restore ETMv4 context across CPU low power states
From: Andrew Murray @ 2019-08-16 15:46 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Alexander Shishkin
  Cc: Al.Grant, coresight, Leo Yan, Sudeep Holla, linux-arm-kernel,
	Mike Leach

Some hardware will ignore bit TRCPDCR.PU which is used to signal
to hardware that power should not be removed from the trace unit.
Let's mitigate against this by conditionally saving and restoring
the trace unit state when the CPU enters low power states.

This patchset introduces a firmware property named
'arm,coresight-loses-context-with-cpu' - when this is present the
hardware state will be conditionally saved and restored.

A module parameter 'pm_save_enable' is also introduced which can
be configured to override the firmware property. This parameter
also provides a means to save/restore state when external agents
are used.

The hardware state is only ever saved and restored when a coresight
session is present.

The last patch should be considered as an RFC as further consideration is
required relating to where the pm_save_enable parameter lives and to
determine if the external agent support should be a pm_save_enable option
or a new kernel option.

Changes since v4:

 - Rename fwnode property to "arm,coresight-loses-context-with-cpu" as this
   doesn't imply a software policy

 - Update the device tree binding document to indicate that this property
   isn't specific to ETMs - also provide a longer description more generic
   description with an example of why it might be used

 - Set the module parameter at probe based on the value determined by firmware.
   The user can still override the firmware via the kernel command line, this
   has the effect of hiding the PARAM_PM_SAVE_FIRMWARE option from the user -
   though we still internally use it to allow us to determine if the user has
   set the parameter.

 - Remove unnecessary call to smp_processor_id

 - Move etm4_needs_save_restore helper to coresight.c and rename

 - Rebased onto coresight/next a04d8683f577 ("...ity of etm4_os_unlock comment")

 - Drop Reviewed-By from Suzuki on "coresight: etm4x: save/restore st..." patch
   as content changed too much

 - Add module option to that keeps clocks/power enabled at probe and saves
   state when external or self-hosted is in use.

Changes since v3:

 - Only save/restore when self-hosted is being used and detect this
   without relying on the coresight registers (which may not be
   available)

 - Only allocate memory for etmv4_save_state at probe time when
   configuration indicates it may be used

 - Set pm_save_enable param to 0444 such that it is static after
   boot

 - Save/restore TRCPDCR

 - Add missing comments to struct etm4_drvdata documentation

 - Rebased onto coresight/next (8f1f9857)

Changes since v2:

 - Move the PM notifier block from drvdata to file static

 - Add section names to document references

 - Add additional information to commit messages

 - Remove trcdvcvr and trcdvcmr from saved state and add a comment to
   describe why

 - Ensure TRCPDCR_PU is set after restore and add a comment to explain
   why we bother toggling TRCPDCR_PU on save/restore

 - Reword the pm_save_enable options and add comments

 - Miscellaneous style changes

 - Move device tree binding documentation to its own patch

Changes since v1:

 - Rebased onto coresight/next

 - Correcly pass bit number rather than BIT macro to coresight_timeout

 - Abort saving state if a timeout occurs

 - Fix completely broken pm_notify handling and unregister handler on error

 - Use state_needs_restore to ensure state is restored only once

 - Add module parameter description to existing boot_enable parameter
   and use module_param instead of module_param_named

 - Add firmware bindings for coresight-needs-save-restore

 - Rename 'disable_pm_save' to 'pm_save_enable' which allows for
   disabled, enabled or firmware

 - Update comment on etm4_os_lock, it incorrectly indicated that
   the code unlocks the trace registers

 - Add comments to explain use of OS lock during save/restore

 - Fix incorrect error description whilst waiting for PM stable

 - Add WARN_ON_ONCE when cpu isn't as expected during save/restore

 - Various updates to commit messages


Andrew Murray (3):
  coresight: etm4x: save/restore state across CPU low power states
  dt-bindings: arm: coresight: Add support for
    coresight-loses-context-with-cpu
  coresight: etm4x: save/restore state for external agents

 .../devicetree/bindings/arm/coresight.txt     |   9 +
 drivers/hwtracing/coresight/coresight-etm4x.c | 339 +++++++++++++++++-
 drivers/hwtracing/coresight/coresight-etm4x.h |  64 ++++
 drivers/hwtracing/coresight/coresight.c       |   8 +-
 include/linux/coresight.h                     |  13 +
 5 files changed, 431 insertions(+), 2 deletions(-)

-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/7] [RFC] ARM: remove Intel iop33x and iop13xx support
From: Aaro Koskinen @ 2019-08-16 15:42 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Vinod Koul, Peter Teichmann, Arnd Bergmann, Bartosz Golaszewski,
	Linux Kernel Mailing List, Russell King, open list:GPIO SUBSYSTEM,
	soc, linux-i2c, dmaengine, Dan Williams, Martin Michlmayr,
	Linux ARM
In-Reply-To: <CACRpkdao8LF8g5qi_h+9BT9cHwmB4OadabkdGfP0sEFeLbmiLw@mail.gmail.com>

Hi,

On Wed, Aug 14, 2019 at 10:36:01AM +0200, Linus Walleij wrote:
> On Mon, Aug 12, 2019 at 11:45 AM Martin Michlmayr <tbm@cyrius.com> wrote:
> > As Arnd points out, Debian used to have support for various iop32x
> > devices.  While Debian hasn't supported iop32x in a number of years,
> > these devices are still usable and in use (RMK being a prime example).
> 
> I suppose it could be a good idea to add support for iop32x to
> OpenWrt and/or OpenEmbedded, both of which support some
> pretty constrained systems.

This platform is not really too constrained... E.g. on N2100 you have
512 MB RAM, SATA disks and gigabit ethernet. Not that different from
mvebu that Debian currently (?) supports. Maybe with multiplatform they
could support iop32x again.

A.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* RE: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC
From: Xiaowei Bao @ 2019-08-16 15:11 UTC (permalink / raw)
  To: Andrew Murray
  Cc: mark.rutland@arm.com, Roy Zang, lorenzo.pieralisi@arm.com,
	arnd@arndb.de, gregkh@linuxfoundation.org, jingoohan1@gmail.com,
	Z.q. Hou, linuxppc-dev@lists.ozlabs.org,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	kishon@ti.com, M.h. Lian, devicetree@vger.kernel.org,
	gustavo.pimentel@synopsys.com, Leo Li, shawnguo@kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190816123524.GE14111@e119886-lin.cambridge.arm.com>



> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年8月16日 20:35
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
> mark.rutland@arm.com; shawnguo@kernel.org; Leo Li
> <leoyang.li@nxp.com>; kishon@ti.com; lorenzo.pieralisi@arm.com;
> arnd@arndb.de; gregkh@linuxfoundation.org; M.h. Lian
> <minghuan.lian@nxp.com>; Roy Zang <roy.zang@nxp.com>;
> linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linuxppc-dev@lists.ozlabs.org; Z.q. Hou <zhiqiang.hou@nxp.com>
> Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for
> DWC
> 
> On Fri, Aug 16, 2019 at 11:00:01AM +0000, Xiaowei Bao wrote:
> >
> >
> > > -----Original Message-----
> > > From: Andrew Murray <andrew.murray@arm.com>
> > > Sent: 2019年8月16日 17:45
> > > To: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
> > > mark.rutland@arm.com; shawnguo@kernel.org; Leo Li
> > > <leoyang.li@nxp.com>; kishon@ti.com; lorenzo.pieralisi@arm.com;
> > > arnd@arndb.de; gregkh@linuxfoundation.org; M.h. Lian
> > > <minghuan.lian@nxp.com>; Roy Zang <roy.zang@nxp.com>;
> > > linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > linuxppc-dev@lists.ozlabs.org; Z.q. Hou <zhiqiang.hou@nxp.com>
> > > Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs
> > > support for DWC
> > >
> > > On Fri, Aug 16, 2019 at 02:55:41AM +0000, Xiaowei Bao wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Andrew Murray <andrew.murray@arm.com>
> > > > > Sent: 2019年8月15日 19:32
> > > > > To: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > > > Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
> > > > > bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> > > > > shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> > > > > lorenzo.pieralisi@arm.com; arnd@arndb.de;
> > > > > gregkh@linuxfoundation.org; M.h. Lian <minghuan.lian@nxp.com>;
> > > > > Mingkai Hu <mingkai.hu@nxp.com>; Roy Zang <roy.zang@nxp.com>;
> > > > > linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> > > > > linux-kernel@vger.kernel.org;
> > > > > linux-arm-kernel@lists.infradead.org;
> > > > > linuxppc-dev@lists.ozlabs.org
> > > > > Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs
> > > > > support for DWC
> > > > >
> > > > > On Thu, Aug 15, 2019 at 04:37:07PM +0800, Xiaowei Bao wrote:
> > > > > > Add multiple PFs support for DWC, different PF have different
> > > > > > config space, we use pf-offset property which get from the DTS
> > > > > > to access the different pF config space.
> > > > >
> > > > > Thanks for the patch. I haven't seen a cover letter for this
> > > > > series, is there one missing?
> > > > Maybe I miss, I will add you to review next time, thanks a lot for
> > > > your
> > > comments.
> > > > >
> > > > >
> > > > > >
> > > > > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > > > > ---
> > > > > >  drivers/pci/controller/dwc/pcie-designware-ep.c |  97
> > > > > +++++++++++++---------
> > > > > >  drivers/pci/controller/dwc/pcie-designware.c    | 105
> > > > > ++++++++++++++++++++++--
> > > > > >  drivers/pci/controller/dwc/pcie-designware.h    |  10 ++-
> > > > > >  include/linux/pci-epc.h                         |   1 +
> > > > > >  4 files changed, 164 insertions(+), 49 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > > index 2bf5a35..75e2955 100644
> > > > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > > @@ -19,12 +19,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep
> > > *ep)
> > > > > >  	pci_epc_linkup(epc);
> > > > > >  }
> > > > > >
> > > > > > -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum
> > > > > > pci_barno
> > > > > bar,
> > > > > > -				   int flags)
> > > > > > +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8
> func_no,
> > > > > > +				   enum pci_barno bar, int flags)
> > > > > >  {
> > > > > >  	u32 reg;
> > > > > > +	struct pci_epc *epc = pci->ep.epc;
> > > > > > +	u32 pf_base = func_no * epc->pf_offset;
> > > > > >
> > > > > > -	reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > > > > > +	reg = pf_base + PCI_BASE_ADDRESS_0 + (4 * bar);
> > > > >
> > > > > I think I'd rather see this arithmetic (and the one for
> > > > > determining
> > > > > pf_base) inside a macro or inline header function. This would
> > > > > make this code more readable and reduce the chances of an error
> > > > > by avoiding
> > > duplication of code.
> > > > >
> > > > > For example look at cdns_pcie_ep_fn_writeb and
> > > > > ROCKCHIP_PCIE_EP_FUNC_BASE for examples of other EP drivers that
> > > > > do this.
> > > > Agree, this looks fine, thanks a lot for your comments, I will use
> > > > this way to access the registers in next version patch.
> > > > >
> > > > >
> > > > > >  	dw_pcie_dbi_ro_wr_en(pci);
> > > > > >  	dw_pcie_writel_dbi2(pci, reg, 0x0);
> > > > > >  	dw_pcie_writel_dbi(pci, reg, 0x0); @@ -37,7 +39,12 @@ static
> > > > > > void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum
> > > > > > pci_barno bar,
> > > > > >
> > > > > >  void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno
> > > > > > bar)
> > > {
> > > > > > -	__dw_pcie_ep_reset_bar(pci, bar, 0);
> > > > > > +	u8 func_no, funcs;
> > > > > > +
> > > > > > +	funcs = pci->ep.epc->max_functions;
> > > > > > +
> > > > > > +	for (func_no = 0; func_no < funcs; func_no++)
> > > > > > +		__dw_pcie_ep_reset_bar(pci, func_no, bar, 0);
> > > > > >  }
> > > > > >
> > > > > >  static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8
> > > > > > cap_ptr, @@ -78,28 +85,29 @@ static int
> > > > > > dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no,  {
> > > > > >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > > > >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > > +	u32 pf_base = func_no * epc->pf_offset;
> > > > > >
> > > > > >  	dw_pcie_dbi_ro_wr_en(pci);
> > > > > > -	dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, hdr->vendorid);
> > > > > > -	dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, hdr->deviceid);
> > > > > > -	dw_pcie_writeb_dbi(pci, PCI_REVISION_ID, hdr->revid);
> > > > > > -	dw_pcie_writeb_dbi(pci, PCI_CLASS_PROG, hdr->progif_code);
> > > > > > -	dw_pcie_writew_dbi(pci, PCI_CLASS_DEVICE,
> > > > > > +	dw_pcie_writew_dbi(pci, pf_base + PCI_VENDOR_ID,
> > > hdr->vendorid);
> > > > > > +	dw_pcie_writew_dbi(pci, pf_base + PCI_DEVICE_ID,
> hdr->deviceid);
> > > > > > +	dw_pcie_writeb_dbi(pci, pf_base + PCI_REVISION_ID,
> hdr->revid);
> > > > > > +	dw_pcie_writeb_dbi(pci, pf_base + PCI_CLASS_PROG,
> > > hdr->progif_code);
> > > > > > +	dw_pcie_writew_dbi(pci, pf_base + PCI_CLASS_DEVICE,
> > > > > >  			   hdr->subclass_code | hdr->baseclass_code << 8);
> > > > > > -	dw_pcie_writeb_dbi(pci, PCI_CACHE_LINE_SIZE,
> > > > > > +	dw_pcie_writeb_dbi(pci, pf_base + PCI_CACHE_LINE_SIZE,
> > > > > >  			   hdr->cache_line_size);
> > > > > > -	dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_VENDOR_ID,
> > > > > > +	dw_pcie_writew_dbi(pci, pf_base +
> PCI_SUBSYSTEM_VENDOR_ID,
> > > > > >  			   hdr->subsys_vendor_id);
> > > > > > -	dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_ID,
> hdr->subsys_id);
> > > > > > -	dw_pcie_writeb_dbi(pci, PCI_INTERRUPT_PIN,
> > > > > > +	dw_pcie_writew_dbi(pci, pf_base + PCI_SUBSYSTEM_ID,
> > > > > hdr->subsys_id);
> > > > > > +	dw_pcie_writeb_dbi(pci, pf_base + PCI_INTERRUPT_PIN,
> > > > > >  			   hdr->interrupt_pin);
> > > > > >  	dw_pcie_dbi_ro_wr_dis(pci);
> > > > > >
> > > > > >  	return 0;
> > > > > >  }
> > > > > >
> > > > > > -static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, enum
> > > > > pci_barno bar,
> > > > > > -				  dma_addr_t cpu_addr,
> > > > > > +static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8
> > > func_no,
> > > > > > +				  enum pci_barno bar, dma_addr_t cpu_addr,
> > > > > >  				  enum dw_pcie_as_type as_type)  {
> > > > > >  	int ret;
> > > > > > @@ -112,7 +120,7 @@ static int dw_pcie_ep_inbound_atu(struct
> > > > > dw_pcie_ep *ep, enum pci_barno bar,
> > > > > >  		return -EINVAL;
> > > > > >  	}
> > > > > >
> > > > > > -	ret = dw_pcie_prog_inbound_atu(pci, free_win, bar, cpu_addr,
> > > > > > +	ret = dw_pcie_prog_inbound_atu(pci, func_no, free_win, bar,
> > > > > > +cpu_addr,
> > > > > >  				       as_type);
> > > > > >  	if (ret < 0) {
> > > > > >  		dev_err(pci->dev, "Failed to program IB window\n"); @@
> > > -125,7
> > > > > > +133,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep
> > > > > > +*ep,
> > > > > enum pci_barno bar,
> > > > > >  	return 0;
> > > > > >  }
> > > > > >
> > > > > > -static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep,
> > > > > > phys_addr_t phys_addr,
> > > > > > +static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8
> > > func_no,
> > > > > > +				   phys_addr_t phys_addr,
> > > > > >  				   u64 pci_addr, size_t size)  {
> > > > > >  	u32 free_win;
> > > > > > @@ -137,8 +146,8 @@ static int dw_pcie_ep_outbound_atu(struct
> > > > > dw_pcie_ep *ep, phys_addr_t phys_addr,
> > > > > >  		return -EINVAL;
> > > > > >  	}
> > > > > >
> > > > > > -	dw_pcie_prog_outbound_atu(pci, free_win,
> PCIE_ATU_TYPE_MEM,
> > > > > > -				  phys_addr, pci_addr, size);
> > > > > > +	dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win,
> > > > > PCIE_ATU_TYPE_MEM,
> > > > > > +				     phys_addr, pci_addr, size);
> > > > > >
> > > > > >  	set_bit(free_win, ep->ob_window_map);
> > > > > >  	ep->outbound_addr[free_win] = phys_addr; @@ -154,7 +163,7
> > > @@
> > > > > static
> > > > > > void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no,
> > > > > >  	enum pci_barno bar = epf_bar->barno;
> > > > > >  	u32 atu_index = ep->bar_to_atu[bar];
> > > > > >
> > > > > > -	__dw_pcie_ep_reset_bar(pci, bar, epf_bar->flags);
> > > > > > +	__dw_pcie_ep_reset_bar(pci, func_no, bar, epf_bar->flags);
> > > > > >
> > > > > >  	dw_pcie_disable_atu(pci, atu_index,
> > > DW_PCIE_REGION_INBOUND);
> > > > > >  	clear_bit(atu_index, ep->ib_window_map); @@ -170,14
> +179,16
> > > @@
> > > > > > static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
> > > > > >  	size_t size = epf_bar->size;
> > > > > >  	int flags = epf_bar->flags;
> > > > > >  	enum dw_pcie_as_type as_type;
> > > > > > -	u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > > > > > +	u32 pf_base = func_no * epc->pf_offset;
> > > > > > +	u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar) + pf_base;
> > > > > >
> > > > > >  	if (!(flags & PCI_BASE_ADDRESS_SPACE))
> > > > > >  		as_type = DW_PCIE_AS_MEM;
> > > > > >  	else
> > > > > >  		as_type = DW_PCIE_AS_IO;
> > > > > >
> > > > > > -	ret = dw_pcie_ep_inbound_atu(ep, bar, epf_bar->phys_addr,
> > > as_type);
> > > > > > +	ret = dw_pcie_ep_inbound_atu(ep, func_no, bar,
> > > > > > +				     epf_bar->phys_addr, as_type);
> > > > > >  	if (ret)
> > > > > >  		return ret;
> > > > > >
> > > > > > @@ -235,7 +246,7 @@ static int dw_pcie_ep_map_addr(struct
> > > > > > pci_epc
> > > > > *epc, u8 func_no,
> > > > > >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > > > >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > >
> > > > > > -	ret = dw_pcie_ep_outbound_atu(ep, addr, pci_addr, size);
> > > > > > +	ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr,
> > > > > > +size);
> > > > > >  	if (ret) {
> > > > > >  		dev_err(pci->dev, "Failed to enable address\n");
> > > > > >  		return ret;
> > > > > > @@ -248,12 +259,13 @@ static int dw_pcie_ep_get_msi(struct
> > > > > > pci_epc *epc, u8 func_no)  {
> > > > > >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > > > >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > > +	u32 pf_base = func_no * epc->pf_offset;
> > > > > >  	u32 val, reg;
> > > > > >
> > > > > >  	if (!ep->msi_cap)
> > > > > >  		return -EINVAL;
> > > > > >
> > > > > > -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> > > > > > +	reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
> > > > > >  	val = dw_pcie_readw_dbi(pci, reg);
> > > > > >  	if (!(val & PCI_MSI_FLAGS_ENABLE))
> > > > > >  		return -EINVAL;
> > > > > > @@ -267,12 +279,13 @@ static int dw_pcie_ep_set_msi(struct
> > > > > > pci_epc *epc, u8 func_no, u8 interrupts)  {
> > > > > >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > > > >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > > +	u32 pf_base = func_no * epc->pf_offset;
> > > > > >  	u32 val, reg;
> > > > > >
> > > > > >  	if (!ep->msi_cap)
> > > > > >  		return -EINVAL;
> > > > > >
> > > > > > -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> > > > > > +	reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
> > > > > >  	val = dw_pcie_readw_dbi(pci, reg);
> > > > > >  	val &= ~PCI_MSI_FLAGS_QMASK;
> > > > > >  	val |= (interrupts << 1) & PCI_MSI_FLAGS_QMASK; @@
> -287,12
> > > > > > +300,13 @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc,
> > > > > > +u8
> > > func_no)  {
> > > > > >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > > > >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > > +	u32 pf_base = func_no * epc->pf_offset;
> > > > > >  	u32 val, reg;
> > > > > >
> > > > > >  	if (!ep->msix_cap)
> > > > > >  		return -EINVAL;
> > > > > >
> > > > > > -	reg = ep->msix_cap + PCI_MSIX_FLAGS;
> > > > > > +	reg = ep->msix_cap + pf_base + PCI_MSIX_FLAGS;
> > > > > >  	val = dw_pcie_readw_dbi(pci, reg);
> > > > > >  	if (!(val & PCI_MSIX_FLAGS_ENABLE))
> > > > > >  		return -EINVAL;
> > > > > > @@ -306,12 +320,13 @@ static int dw_pcie_ep_set_msix(struct
> > > > > > pci_epc *epc, u8 func_no, u16 interrupts)  {
> > > > > >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > > > >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > > +	u32 pf_base = func_no * epc->pf_offset;
> > > > > >  	u32 val, reg;
> > > > > >
> > > > > >  	if (!ep->msix_cap)
> > > > > >  		return -EINVAL;
> > > > > >
> > > > > > -	reg = ep->msix_cap + PCI_MSIX_FLAGS;
> > > > > > +	reg = ep->msix_cap + pf_base + PCI_MSIX_FLAGS;
> > > > > >  	val = dw_pcie_readw_dbi(pci, reg);
> > > > > >  	val &= ~PCI_MSIX_FLAGS_QSIZE;
> > > > > >  	val |= interrupts;
> > > > > > @@ -400,6 +415,7 @@ int dw_pcie_ep_raise_msi_irq(struct
> > > dw_pcie_ep
> > > > > *ep, u8 func_no,
> > > > > >  	unsigned int aligned_offset;
> > > > > >  	u16 msg_ctrl, msg_data;
> > > > > >  	u32 msg_addr_lower, msg_addr_upper, reg;
> > > > > > +	u32 pf_base = func_no * epc->pf_offset;
> > > > > >  	u64 msg_addr;
> > > > > >  	bool has_upper;
> > > > > >  	int ret;
> > > > > > @@ -408,19 +424,19 @@ int dw_pcie_ep_raise_msi_irq(struct
> > > > > dw_pcie_ep *ep, u8 func_no,
> > > > > >  		return -EINVAL;
> > > > > >
> > > > > >  	/* Raise MSI per the PCI Local Bus Specification Revision 3.0,
> 6.8.1.
> > > */
> > > > > > -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> > > > > > +	reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
> > > > > >  	msg_ctrl = dw_pcie_readw_dbi(pci, reg);
> > > > > >  	has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
> > > > > > -	reg = ep->msi_cap + PCI_MSI_ADDRESS_LO;
> > > > > > +	reg = ep->msi_cap + pf_base + PCI_MSI_ADDRESS_LO;
> > > > > >  	msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
> > > > > >  	if (has_upper) {
> > > > > > -		reg = ep->msi_cap + PCI_MSI_ADDRESS_HI;
> > > > > > +		reg = ep->msi_cap + pf_base + PCI_MSI_ADDRESS_HI;
> > > > > >  		msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
> > > > > > -		reg = ep->msi_cap + PCI_MSI_DATA_64;
> > > > > > +		reg = ep->msi_cap + pf_base + PCI_MSI_DATA_64;
> > > > > >  		msg_data = dw_pcie_readw_dbi(pci, reg);
> > > > > >  	} else {
> > > > > >  		msg_addr_upper = 0;
> > > > > > -		reg = ep->msi_cap + PCI_MSI_DATA_32;
> > > > > > +		reg = ep->msi_cap + pf_base + PCI_MSI_DATA_32;
> > > > > >  		msg_data = dw_pcie_readw_dbi(pci, reg);
> > > > > >  	}
> > > > > >  	aligned_offset = msg_addr_lower & (epc->mem->page_size -
> 1);
> > > @@
> > > > > > -439,7 +455,7 @@ int dw_pcie_ep_raise_msi_irq(struct
> > > > > > dw_pcie_ep *ep,
> > > > > > u8 func_no,  }
> > > > > >
> > > > > >  int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8
> func_no,
> > > > > > -			     u16 interrupt_num)
> > > > > > +			      u16 interrupt_num)
> > > > > >  {
> > > > > >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > >  	struct pci_epc *epc = ep->epc; @@ -447,16 +463,17 @@ int
> > > > > > dw_pcie_ep_raise_msix_irq(struct
> > > > > dw_pcie_ep *ep, u8 func_no,
> > > > > >  	u32 bar_addr_upper, bar_addr_lower;
> > > > > >  	u32 msg_addr_upper, msg_addr_lower;
> > > > > >  	u32 reg, msg_data, vec_ctrl;
> > > > > > +	u32 pf_base = func_no * epc->pf_offset;
> > > > > >  	u64 tbl_addr, msg_addr, reg_u64;
> > > > > >  	void __iomem *msix_tbl;
> > > > > >  	int ret;
> > > > > >
> > > > > > -	reg = ep->msix_cap + PCI_MSIX_TABLE;
> > > > > > +	reg = ep->msix_cap + pf_base + PCI_MSIX_TABLE;
> > > > > >  	tbl_offset = dw_pcie_readl_dbi(pci, reg);
> > > > > >  	bir = (tbl_offset & PCI_MSIX_TABLE_BIR);
> > > > > >  	tbl_offset &= PCI_MSIX_TABLE_OFFSET;
> > > > > >
> > > > > > -	reg = PCI_BASE_ADDRESS_0 + (4 * bir);
> > > > > > +	reg = PCI_BASE_ADDRESS_0 + pf_base + (4 * bir);
> > > > > >  	bar_addr_upper = 0;
> > > > > >  	bar_addr_lower = dw_pcie_readl_dbi(pci, reg);
> > > > > >  	reg_u64 = (bar_addr_lower &
> > > PCI_BASE_ADDRESS_MEM_TYPE_MASK);
> > > > > @@
> > > > > > -592,13 +609,17 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > > > > >  	ep->epc = epc;
> > > > > >  	epc_set_drvdata(epc, ep);
> > > > > >
> > > > > > -	if (ep->ops->ep_init)
> > > > > > -		ep->ops->ep_init(ep);
> > > > > > -
> > > > > >  	ret = of_property_read_u8(np, "max-functions",
> > > &epc->max_functions);
> > > > > >  	if (ret < 0)
> > > > > >  		epc->max_functions = 1;
> > > > > >
> > > > > > +	ret = of_property_read_u32(np, "pf-offset", &epc->pf_offset);
> > > > > > +	if (ret < 0)
> > > > > > +		epc->pf_offset = 0;
> > > > >
> > > > > Bad things will likely happen if max_functions > 1 and pf-offset isn't
> set.
> > > > > I think the driver should bail in this situation. It would be
> > > > > very easy for someone to misconfigure this.
> > > > Yes, you are right, but if the max-functions have defined in DTS,
> > > > require the pf-offset must define in DTS, I am not sure the
> > > > correct value of pf-offsetfor other platforms, so I think the
> > > > max-functions and
> > > pf-offset should not have the dependence.
> > >
> > > Yes you're correct. I hadn't really thought about this beyond
> > > layerscape. It's also possible that other hardware could support
> > > multiple PFs without relying on an offset and perhaps employ some
> > > other mechanism to access different functions. So whilst this
> > > property can be optional for the majority of dwc controllers - it must be
> set and cannot be zero for layerscape.
> > >
> > > Perhaps inside ls_pcie_ep_init, you can set max_functions to 1 if
> > > pf_offset is
> > > 0 and print a WARN to explain why? (Or ls_pcie_ep_init returns
> > > failure and dw_pcie_ep_init checks it and bails).
> > >
> > > The assumption is being made here that future dw controllers may
> > > also use pf_offset (is this likely?) - otherwise why is this in
> > > pcie-designware-ep.c and not pci-layerscape-ep.c and why is this value
> not just hard-coded for lp?
> >
> > Thanks a lot for your detail comments, this give me a lot of help.
> > Yes, I agree your point, and I will seriously consider a best way to fix this
> potential issue.
> > Based on your experience, how do other platforms implement the multiple
> functions?
> > The DWC core difference the different PF by signal
> "client0_tlp_func_num[(PF_WD-1):0]"
> 
> I don't know, though looking at the kernel drivers suggests that the existing EP
> controllers have a large address space which contains multiple PFs. They are
> accessed via macros (ROCKCHIP_PCIE_EP_FUNC_BASE(fn),
> CDNS_PCIE_EP_FUNC_BASE(fn)). It would be possible, but probably not
> desirable to have a smaller address space (window) and a register that selects
> which function the window refers to. This is why I'm slight nervous of
> assuming that a pf-offset will cover all future dw drivers - I may be wrong.
OK, thanks, maybe other people have good advice. I will use the macro to implement
the multiple function in v2 patch.
> 
> > >
> > >
> > > > even though I didn't define pf-offset when I defined
> > > > max-functions, the pf-offset is 0, the DWC ep driver can continue
> > > > run the progress of INIT but not return, of course, thus the PF1
> > > > will not work, I don't know which
> > > way is better.
> > Hi Andrew,
> > > > >
> > > > >
> > > > > > +
> > > > > > +	if (ep->ops->ep_init)
> > > > > > +		ep->ops->ep_init(ep);
> > > > > > +
> > > > > >  	ret = __pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
> > > > > >  				 ep->page_size);
> > > > > >  	if (ret < 0) {
> > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > index 7d25102..c99cee4 100644
> > > > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > @@ -158,6 +158,43 @@ static void
> > > > > > dw_pcie_writel_ob_unroll(struct
> > > > > dw_pcie *pci, u32 index, u32 reg,
> > > > > >  	dw_pcie_writel_atu(pci, offset + reg, val);  }
> > > > > >
> > > > > > +static void dw_pcie_prog_ep_outbound_atu_unroll(struct
> > > > > > +dw_pcie *pci, u8
> > > > > func_no,
> > > > > > +						int index, int type,
> > > > > > +						u64 cpu_addr, u64 pci_addr,
> > > > > > +						u32 size)
> > > > > > +{
> > > > > > +	u32 retries, val;
> > > > > > +
> > > > > > +	dw_pcie_writel_ob_unroll(pci, index,
> > > PCIE_ATU_UNR_LOWER_BASE,
> > > > > > +				 lower_32_bits(cpu_addr));
> > > > > > +	dw_pcie_writel_ob_unroll(pci, index,
> > > PCIE_ATU_UNR_UPPER_BASE,
> > > > > > +				 upper_32_bits(cpu_addr));
> > > > > > +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LIMIT,
> > > > > > +				 lower_32_bits(cpu_addr + size - 1));
> > > > > > +	dw_pcie_writel_ob_unroll(pci, index,
> > > PCIE_ATU_UNR_LOWER_TARGET,
> > > > > > +				 lower_32_bits(pci_addr));
> > > > > > +	dw_pcie_writel_ob_unroll(pci, index,
> > > PCIE_ATU_UNR_UPPER_TARGET,
> > > > > > +				 upper_32_bits(pci_addr));
> > > > > > +	dw_pcie_writel_ob_unroll(pci, index,
> > > PCIE_ATU_UNR_REGION_CTRL1,
> > > > > > +				 type | PCIE_ATU_FUNC_NUM(func_no));
> > > > >
> > > > > With the exception of this line, the rest of this function is
> > > > > identical to dw_pcie_prog_outbound_atu_unroll.
> > > > Yes, I can integrate the same code, but I think we'd better use
> > > > the different outbound window set function between RC and EP,
> > > > because the RC
> > > don't need the func_num parameter.
> > >
> > >
> > >
> > > > >
> > > > > > +	dw_pcie_writel_ob_unroll(pci, index,
> > > PCIE_ATU_UNR_REGION_CTRL2,
> > > > > > +				 PCIE_ATU_ENABLE);
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * Make sure ATU enable takes effect before any subsequent
> config
> > > > > > +	 * and I/O accesses.
> > > > > > +	 */
> > > > > > +	for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES;
> > > > > > +retries++)
> > > {
> > > > > > +		val = dw_pcie_readl_ob_unroll(pci, index,
> > > > > > +					      PCIE_ATU_UNR_REGION_CTRL2);
> > > > > > +		if (val & PCIE_ATU_ENABLE)
> > > > > > +			return;
> > > > > > +
> > > > > > +		mdelay(LINK_WAIT_IATU);
> > > > > > +	}
> > > > > > +	dev_err(pci->dev, "Outbound iATU is not being enabled\n"); }
> > > > > > +
> > > > > >  static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie
> > > > > > *pci, int
> > > > > index,
> > > > > >  					     int type, u64 cpu_addr,
> > > > > >  					     u64 pci_addr, u32 size) @@ -194,6
> > > +231,51 @@ static
> > > > > > void
> > > > > dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
> > > > > >  	dev_err(pci->dev, "Outbound iATU is not being enabled\n");
> > > > > > }
> > > > > >
> > > > > > +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8
> > > > > > +func_no, int
> > > > > index,
> > > > > > +				  int type, u64 cpu_addr, u64 pci_addr,
> > > > > > +				  u32 size)
> > > > > > +{
> > > > > > +	u32 retries, val;
> > > > > > +
> > > > > > +	if (pci->ops->cpu_addr_fixup)
> > > > > > +		cpu_addr = pci->ops->cpu_addr_fixup(pci, cpu_addr);
> > > > > > +
> > > > > > +	if (pci->iatu_unroll_enabled) {
> > > > > > +		dw_pcie_prog_ep_outbound_atu_unroll(pci, func_no,
> index,
> > > type,
> > > > > > +						    cpu_addr, pci_addr, size);
> > > > > > +		return;
> > > > > > +	}
> > > > > > +
> > > > > > +	dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT,
> > > > > > +			   PCIE_ATU_REGION_OUTBOUND | index);
> > > > > > +	dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_BASE,
> > > > > > +			   lower_32_bits(cpu_addr));
> > > > > > +	dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_BASE,
> > > > > > +			   upper_32_bits(cpu_addr));
> > > > > > +	dw_pcie_writel_dbi(pci, PCIE_ATU_LIMIT,
> > > > > > +			   lower_32_bits(cpu_addr + size - 1));
> > > > > > +	dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_TARGET,
> > > > > > +			   lower_32_bits(pci_addr));
> > > > > > +	dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_TARGET,
> > > > > > +			   upper_32_bits(pci_addr));
> > > > > > +	dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
> > > > > > +			   PCIE_ATU_FUNC_NUM(func_no));
> > > > >
> > > > > The same here, this is identical to dw_pcie_prog_outbound_atu
> > > > > with the exception of this line.
> > > > >
> > > > > Is there a way you can avoid all of this duplicated code?
> > > > As above, I can integrate the same code, but I keep to think the
> > > > different outbound Window set function should be used between RC and
> EP.
> > >
> > > Or, is it possible to keep and use the existing functions, but use
> > > them differently, e.g:
> > >
> > >
> > > @@ -137,8 +146,8 @@ static int dw_pcie_ep_outbound_atu(struct
> > > dw_pcie_ep *ep, phys_addr_t phys_addr,
> > >                 return -EINVAL;
> > >         }
> > >
> > > -       dw_pcie_prog_outbound_atu(pci, free_win,
> PCIE_ATU_TYPE_MEM,
> > > -                                 phys_addr, pci_addr, size);
> > > +       dw_pcie_prog_outbound_atu(pci, free_win,
> > > PCIE_ATU_TYPE_MEM_FUNC(func_no),
> > > +                                    phys_addr, pci_addr, size);
> > >
> > >         set_bit(free_win, ep->ob_window_map);
> > >         ep->outbound_addr[free_win] = phys_addr;
> > >
> > >
> > > Supported with:
> > >
> > > #define PCIE_ATU_TYPE_MEM               0x0
> > > #define PCIE_ATU_TYPE_MEM_FUNC(func_no) (PCIE_ATU_TYPE_MEM |
> > > PCIE_ATU_FUNC_NUM(func_no))
> > >
> > >
> > > This is just a suggestion, but I'm keen to avoid code duplication.
> > Thanks, I have think of a way as follow:
> >
> > This is a good way, but I think PCIE_ATU_TYPE_MEM_FUNC(func_no) will
> > give Someone confused meaning, because PCIE_ATU_TYPE_MEM indicate
> the
> > type of TLP, and the location in the bit[0:3] of register CR1, but the
> > PCIE_ATU_FUNC_NUM is bit[20:24], I have another way:
> > @@ -137,8 +146,8 @@ static int dw_pcie_ep_outbound_atu(struct
> > dw_pcie_ep *ep, phys_addr_t phys_addr,
> >                  return -EINVAL;
> >         }
> >
> > 		dw_pcie_prog_outbound_atu(pci, free_win, PCIE_ATU_TYPE_MEM,
> >                                 phys_addr, pci_addr, size);
> > +		val = dw_pcie_readl_dbi(pci, PCIE_ATU_CR1);
> > +		dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, val |
> > +PCIE_ATU_FUNC_NUM(func_no));
> > or
> > +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int
> index,
> > +                                   int type, u64 cpu_addr, u64
> pci_addr,
> > +                                   u32 size) {
> > +		dw_pcie_prog_outbound_atu(pci, index, type, cpu_addr, pci_addr,
> size);
> > +		val = dw_pcie_readl_dbi(pci, PCIE_ATU_CR1);
> > +		dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, val |
> > +PCIE_ATU_FUNC_NUM(func_no)); }
> >
> > Which do you think is better of these three ways?
> 
> Building upon your idea, how about:
> 
> 
>  @@ -137,8 +146,8 @@ static int dw_pcie_ep_outbound_atu(struct
> dw_pcie_ep *ep, phys_addr_t phys_addr,
>                  return -EINVAL;
>          }
> 
>  -       dw_pcie_prog_outbound_atu(pci, free_win,
> PCIE_ATU_TYPE_MEM,
>  -                                 phys_addr, pci_addr, size);
>  +       dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win,
> PCIE_ATU_TYPE_MEM
>  +                                    phys_addr, pci_addr, size);
> 
>          set_bit(free_win, ep->ob_window_map);
>          ep->outbound_addr[free_win] = phys_addr;
> 
> 
>  +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int
> index,
>  +                                   int type, u64 cpu_addr, u64
> pci_addr,
>  +                                   u32 size)
>  +{
>  +		__dw_pcie_prog_outbound_atu(pci, func_no, index, type, cpu_addr,
> pci_addr, size);
>  +}
>  +
>  +void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, int
> index,
>  +                                   int type, u64 cpu_addr, u64
> pci_addr,
>  +                                   u32 size)
>  +{
>  +		__dw_pcie_prog_outbound_atu(pci, 0, index, type, cpu_addr,
> pci_addr, size);
>  +}
> 
> In other words dw_pcie_prog_outbound_atu is updated (and renamed) to
> always take a func_no and for host controllers this is always set to zero. Or
> you could follow the approach taken in the cadence drivers for their
> implementation of cdns_pcie_set_outbound_region - this always takes a
> func_no and is used by host controller and endpoint drivers (except they don't
> have the helper wrapper functions above thus exposing fn=0 to host
> controllers).
You're correct, I think this way is better, thanks.
> 
> > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Andrew Murray
> > > > >
> > > > > > +	dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE);
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * Make sure ATU enable takes effect before any subsequent
> config
> > > > > > +	 * and I/O accesses.
> > > > > > +	 */
> > > > > > +	for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES;
> > > > > > +retries++)
> > > {
> > > > > > +		val = dw_pcie_readl_dbi(pci, PCIE_ATU_CR2);
> > > > > > +		if (val & PCIE_ATU_ENABLE)
> > > > > > +			return;
> > > > > > +
> > > > > > +		mdelay(LINK_WAIT_IATU);
> > > > > > +	}
> > > > > > +	dev_err(pci->dev, "Outbound iATU is not being enabled\n"); }
> > > > > > +
> > > > > >  void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int
> > > > > > index, int
> > > type,
> > > > > >  			       u64 cpu_addr, u64 pci_addr, u32 size)
> { @@
> > > -252,8
> > > > > +334,8
> > > > > > @@ static void dw_pcie_writel_ib_unroll(struct dw_pcie *pci,
> > > > > > u32 index,
> > > > > u32 reg,
> > > > > >  	dw_pcie_writel_atu(pci, offset + reg, val);  }
> > > > > >
> > > > > > -static int dw_pcie_prog_inbound_atu_unroll(struct dw_pcie
> > > > > > *pci, int
> > > index,
> > > > > > -					   int bar, u64 cpu_addr,
> > > > > > +static int dw_pcie_prog_inbound_atu_unroll(struct dw_pcie
> > > > > > +*pci,
> > > > > > +u8
> > > > > func_no,
> > > > > > +					   int index, int bar, u64 cpu_addr,
> > > > > >  					   enum dw_pcie_as_type as_type)  {
> > > > > >  	int type;
> > > > > > @@ -275,8 +357,10 @@ static int
> > > > > > dw_pcie_prog_inbound_atu_unroll(struct
> > > > > dw_pcie *pci, int index,
> > > > > >  		return -EINVAL;
> > > > > >  	}
> > > > > >
> > > > > > -	dw_pcie_writel_ib_unroll(pci, index,
> > > PCIE_ATU_UNR_REGION_CTRL1,
> > > > > type);
> > > > > > +	dw_pcie_writel_ib_unroll(pci, index,
> > > PCIE_ATU_UNR_REGION_CTRL1,
> > > > > type |
> > > > > > +				 PCIE_ATU_FUNC_NUM(func_no));
> > > > > >  	dw_pcie_writel_ib_unroll(pci, index,
> > > PCIE_ATU_UNR_REGION_CTRL2,
> > > > > > +				 PCIE_ATU_FUNC_NUM_MATCH_EN |
> > > > > >  				 PCIE_ATU_ENABLE |
> > > > > >  				 PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
> > > > > >
> > > > > > @@ -297,14 +381,15 @@ static int
> > > > > dw_pcie_prog_inbound_atu_unroll(struct dw_pcie *pci, int index,
> > > > > >  	return -EBUSY;
> > > > > >  }
> > > > > >
> > > > > > -int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int
> bar,
> > > > > > -			     u64 cpu_addr, enum dw_pcie_as_type
> as_type)
> > > > > > +int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no,
> > > > > > +int
> > > index,
> > > > > > +			     int bar, u64 cpu_addr,
> > > > > > +			     enum dw_pcie_as_type as_type)
> > > > > >  {
> > > > > >  	int type;
> > > > > >  	u32 retries, val;
> > > > > >
> > > > > >  	if (pci->iatu_unroll_enabled)
> > > > > > -		return dw_pcie_prog_inbound_atu_unroll(pci, index, bar,
> > > > > > +		return dw_pcie_prog_inbound_atu_unroll(pci, func_no,
> index,
> > > > > > +bar,
> > > > > >  						       cpu_addr, as_type);
> > > > > >
> > > > > >  	dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT,
> > > > > PCIE_ATU_REGION_INBOUND |
> > > > > > @@ -323,9 +408,11 @@ int dw_pcie_prog_inbound_atu(struct
> > > > > > dw_pcie
> > > > > *pci, int index, int bar,
> > > > > >  		return -EINVAL;
> > > > > >  	}
> > > > > >
> > > > > > -	dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type);
> > > > > > -	dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE
> > > > > > -			   | PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
> > > > > > +	dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
> > > > > > +			   PCIE_ATU_FUNC_NUM(func_no));
> > > > > > +	dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE |
> > > > > > +			   PCIE_ATU_FUNC_NUM_MATCH_EN |
> > > > > > +			   PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
> > > > > >
> > > > > >  	/*
> > > > > >  	 * Make sure ATU enable takes effect before any subsequent
> > > > > > config diff --git
> > > > > > a/drivers/pci/controller/dwc/pcie-designware.h
> > > > > > b/drivers/pci/controller/dwc/pcie-designware.h
> > > > > > index ffed084..2b291e8 100644
> > > > > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > > > > @@ -71,9 +71,11 @@
> > > > > >  #define PCIE_ATU_TYPE_IO		0x2
> > > > > >  #define PCIE_ATU_TYPE_CFG0		0x4
> > > > > >  #define PCIE_ATU_TYPE_CFG1		0x5
> > > > > > +#define PCIE_ATU_FUNC_NUM(pf)           (pf << 20)
> > > > > >  #define PCIE_ATU_CR2			0x908
> > > > > >  #define PCIE_ATU_ENABLE			BIT(31)
> > > > > >  #define PCIE_ATU_BAR_MODE_ENABLE	BIT(30)
> > > > > > +#define PCIE_ATU_FUNC_NUM_MATCH_EN      BIT(19)
> > > > > >  #define PCIE_ATU_LOWER_BASE		0x90C
> > > > > >  #define PCIE_ATU_UPPER_BASE		0x910
> > > > > >  #define PCIE_ATU_LIMIT			0x914
> > > > > > @@ -265,8 +267,12 @@ int dw_pcie_wait_for_link(struct dw_pcie
> > > > > > *pci); void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int
> index,
> > > > > >  			       int type, u64 cpu_addr, u64 pci_addr,
> > > > > >  			       u32 size);
> > > > > > -int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int
> bar,
> > > > > > -			     u64 cpu_addr, enum dw_pcie_as_type
> as_type);
> > > > > > +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8
> > > > > > +func_no, int
> > > > > index,
> > > > > > +				  int type, u64 cpu_addr, u64 pci_addr,
> > > > > > +				  u32 size);
> > > > > > +int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no,
> > > > > > +int
> > > index,
> > > > > > +			     int bar, u64 cpu_addr,
> > > > > > +			     enum dw_pcie_as_type as_type);
> > > > > >  void dw_pcie_disable_atu(struct dw_pcie *pci, int index,
> > > > > >  			 enum dw_pcie_region_type type);  void
> > > dw_pcie_setup(struct
> > > > > > dw_pcie *pci); diff --git a/include/linux/pci-epc.h
> > > > > > b/include/linux/pci-epc.h index f641bad..fc2feee 100644
> > > > > > --- a/include/linux/pci-epc.h
> > > > > > +++ b/include/linux/pci-epc.h
> > > > > > @@ -96,6 +96,7 @@ struct pci_epc {
> > > > > >  	const struct pci_epc_ops	*ops;
> > > > > >  	struct pci_epc_mem		*mem;
> > > > > >  	u8				max_functions;
> > > > > > +	u32				pf_offset;
> > >
> > > Also pf_offset is an implementation detail needed only by the driver
> > > to calculate where the PF is - it doesn't seem right that we share
> > > this with the EP controller framework (whereas max_functions is used
> > > as a bounds check for func_no in the framework calls).
> > >
> > > I'd suggest that pf_offset is moved to a dwc structure, perhaps
> dw_pcie_ep?
> > I add the variable to this struct is consider that all PF is belong to
> > a PCIe controller, and the pci_epc indicate a PCIe controller, so I
> > add this variable to this struct, what do you think about this? I am not sure
> whether I should add this variable to dw_pcie_ep.
> 
> The EPC framework won't use the pf_offset and doesn't need it. It abstracts
> the complexity of writing to the config address space (and similar) through the
> pci_epc_ops. I'd suggest that the EPC framework (and pci_epc struct) only
> needs to contain what *it* needs. Especially given that not all ep drivers have
> a pf_offset or similar.
> 
> I understand the logic that pci_epc represents a EP controller, but I think you
> should consider that it actually represents a *generic* EP controller in the
> context of a framework which solely serves the purpose of connecting
> controllers with functions. Whereas the dw_pcie_ep represents a specific
> type of controller (DW) - as the pf_offset is (so far) relating to only DW
> controllers (and as confirmed by the DT mapping) then it makes sense to not
> move pf_offset from the specialised specific controller to the generic
> controller. (Or at least this is how I rationalise it, though the EPC framework is
> something quite unfamiliar to me).
I think this is more reasonable by your detail explaining, I will move pf_offset
to the struct dw_pcie_ep, thanks again!
> 
> Thanks,
> 
> Andrew Murray
> 
> > >
> > > Thanks,
> > >
> > > Andrew Murray
> > >
> > > > > >  	struct config_group		*group;
> > > > > >  	/* spinlock to protect against concurrent access of EP
> controller */
> > > > > >  	spinlock_t			lock;
> > > > > > --
> > > > > > 2.9.5
> > > > > >
> > > > > >
> > > > > > _______________________________________________
> > > > > > linux-arm-kernel mailing list
> > > > > > linux-arm-kernel@lists.infradead.org
> > > > > > http://lists
> > > > > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&amp;dat
> > > > > > a=0
> > > 2
> > > > > > %
> > > > > 7C0
> > > > > >
> > > > >
> > >
> 1%7Cxiaowei.bao%40nxp.com%7C0e39168f6f144db6840308d721742040%7
> > > > > C686ea1d
> > > > > >
> > > > >
> > >
> 3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637014654998524452&amp;sd
> > > > > ata=bP7eh
> > > > > > cjlGXCMVFE2b4f12Q6fGV7lQ%2F5i9qIi9FoPlbI%3D&amp;reserved=0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: do_csum: implement accelerated scalar version
From: Robin Murphy @ 2019-08-16 14:55 UTC (permalink / raw)
  To: Shaokun Zhang, Will Deacon
  Cc: Ard Biesheuvel, steve.capper, netdev, ilias.apalodimas,
	huanglingyan (A), linux-arm-kernel
In-Reply-To: <37fbc2a3-069d-9f75-f3d0-3eda2efa5c9b@hisilicon.com>

On 16/08/2019 09:15, Shaokun Zhang wrote:
> Hi Will,
> 
> On 2019/8/16 0:46, Will Deacon wrote:
>> On Thu, May 16, 2019 at 11:14:35AM +0800, Zhangshaokun wrote:
>>> On 2019/5/15 17:47, Will Deacon wrote:
>>>> On Mon, Apr 15, 2019 at 07:18:22PM +0100, Robin Murphy wrote:
>>>>> On 12/04/2019 10:52, Will Deacon wrote:
>>>>>> I'm waiting for Robin to come back with numbers for a C implementation.
>>>>>>
>>>>>> Robin -- did you get anywhere with that?
>>>>>
>>>>> Still not what I would call finished, but where I've got so far (besides an
>>>>> increasingly elaborate test rig) is as below - it still wants some unrolling
>>>>> in the middle to really fly (and actual testing on BE), but the worst-case
>>>>> performance already equals or just beats this asm version on Cortex-A53 with
>>>>> GCC 7 (by virtue of being alignment-insensitive and branchless except for
>>>>> the loop). Unfortunately, the advantage of C code being instrumentable does
>>>>> also come around to bite me...
>>>>
>>>> Is there any interest from anybody in spinning a proper patch out of this?
>>>> Shaokun?
>>>
>>> HiSilicon's Kunpeng920(Hi1620) benefits from do_csum optimization, if Ard and
>>> Robin are ok, Lingyan or I can try to do it.
>>> Of course, if any guy posts the patch, we are happy to test it.
>>> Any will be ok.
>>
>> I don't mind who posts it, but Robin is super busy with SMMU stuff at the
>> moment so it probably makes more sense for you or Lingyan to do it.
> 
> Thanks for restarting this topic, I or Lingyan will do it soon.

FWIW, I've rolled up what I had so far and dumped it up into a quick 
semi-realistic patch here:

http://linux-arm.org/git?p=linux-rm.git;a=commitdiff;h=859c5566510c32ae72039aa5072e932a771a3596

So far I'd put most of the effort into the aforementioned benchmarking 
harness to compare performance and correctness for all the proposed 
implementations over all reasonable alignment/length combinations - I 
think that got pretty much finished, but as Will says I'm unlikely to 
find time to properly look at this again for several weeks.

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] ARM64: dts: allwinner: Add devicetree for pine H64 modelA evaluation board
From: Corentin Labbe @ 2019-08-16 14:00 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: mark.rutland, devicetree, linux-sunxi, linux-kernel, wens,
	robh+dt, linux-arm-kernel
In-Reply-To: <20190816135206.pnf3iperzyhcbg4h@flea>

On Fri, Aug 16, 2019 at 03:52:06PM +0200, Maxime Ripard wrote:
> On Fri, Aug 16, 2019 at 01:57:50PM +0200, Corentin Labbe wrote:
> > On Fri, Aug 16, 2019 at 01:36:50PM +0200, Maxime Ripard wrote:
> > > On Fri, Aug 16, 2019 at 11:35:13AM +0200, Corentin Labbe wrote:
> > > > On Wed, Aug 14, 2019 at 03:33:22PM +0200, Maxime Ripard wrote:
> > > > > On Wed, Aug 14, 2019 at 03:17:41PM +0200, Corentin Labbe wrote:
> > > > > > On Mon, Aug 12, 2019 at 11:40:00AM +0200, Maxime Ripard wrote:
> > > > > > > On Thu, Aug 08, 2019 at 10:42:53AM +0200, Corentin Labbe wrote:
> > > > > > > > This patch adds the evaluation variant of the model A of the PineH64.
> > > > > > > > The model A has the same size of the pine64 and has a PCIE slot.
> > > > > > > >
> > > > > > > > The only devicetree difference with current pineH64, is the PHY
> > > > > > > > regulator.
> > > > > > > >
> > > > > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > > > > > > > ---
> > > > > > > >  arch/arm64/boot/dts/allwinner/Makefile        |  1 +
> > > > > > > >  .../sun50i-h6-pine-h64-modelA-eval.dts        | 26 +++++++++++++++++++
> > > > > > > >  2 files changed, 27 insertions(+)
> > > > > > > >  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts
> > > > > > > >
> > > > > > > > diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> > > > > > > > index f6db0611cb85..9a02166cbf72 100644
> > > > > > > > --- a/arch/arm64/boot/dts/allwinner/Makefile
> > > > > > > > +++ b/arch/arm64/boot/dts/allwinner/Makefile
> > > > > > > > @@ -25,3 +25,4 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3.dtb
> > > > > > > >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-lite2.dtb
> > > > > > > >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-one-plus.dtb
> > > > > > > >  dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64.dtb
> > > > > > > > +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64-modelA-eval.dtb
> > > > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts
> > > > > > > > new file mode 100644
> > > > > > > > index 000000000000..d8ff02747efe
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts
> > > > > > > > @@ -0,0 +1,26 @@
> > > > > > > > +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> > > > > > > > +/*
> > > > > > > > + * Copyright (C) 2019 Corentin Labbe <clabbe.montjoie@gmail.com>
> > > > > > > > + */
> > > > > > > > +
> > > > > > > > +#include "sun50i-h6-pine-h64.dts"
> > > > > > > > +
> > > > > > > > +/ {
> > > > > > > > +	model = "Pine H64 model A evaluation board";
> > > > > > > > +	compatible = "pine64,pine-h64-modelA-eval", "allwinner,sun50i-h6";
> > > > > > > > +
> > > > > > > > +	reg_gmac_3v3: gmac-3v3 {
> > > > > > > > +		compatible = "regulator-fixed";
> > > > > > > > +		regulator-name = "vcc-gmac-3v3";
> > > > > > > > +		regulator-min-microvolt = <3300000>;
> > > > > > > > +		regulator-max-microvolt = <3300000>;
> > > > > > > > +		startup-delay-us = <100000>;
> > > > > > > > +		gpio = <&pio 2 16 GPIO_ACTIVE_HIGH>;
> > > > > > > > +		enable-active-high;
> > > > > > > > +	};
> > > > > > > > +
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +&emac {
> > > > > > > > +	phy-supply = <&reg_gmac_3v3>;
> > > > > > > > +};
> > > > > > >
> > > > > > > I might be missing some context here, but I'm pretty sure that the
> > > > > > > initial intent of the pine h64 DTS was to support the model A all
> > > > > > > along.
> > > > > > >
> > > > > >
> > > > > > The regulator changed between modelA and B.
> > > > > > See this old patchset (supporting modelA) https://patchwork.kernel.org/patch/10539149/ for example.
> > > > >
> > > > > I'm not sure what your point is, but mine is that everything about the
> > > > > model A should be in sun50i-h6-pine-h64.dts.
> > > > >
> > > >
> > > > model A and B are different enough for distinct dtb, (see sub-thread
> > > > on HDMI difference for an other difference than PHY regulator)
> > >
> > > I don't mind having separate DTBs for model A and model B.
> > >
> > > > And clearly, the current dtb is for model B.
> > >
> > > That DTS was added almost a year before the model B was announced, and
> > > no commit to that file mention the model B, so it's definitely not
> > > clear.
> >
> > Normal it was added for model A (without any ethernet/HDMI support,
> > so nothing distinct from model B), and the modelB ethernet/HDMI
> > support cames after.
> 
> Changing the board a DT is meant to halfway through the development is
> definitely not ok.
> 
> > > > So do you mean that we need to create a new dtb for model B ? (and
> > > > hack the current back to model A ?)
> > >
> > > I'd prefer not to hack anything, but yes
> > >
> >
> > Since model A is not public (only evaluations boards exists), the
> > probability of a production model A is low and the current dtb is
> > perfect for model B , could you reconsider this ?
> 
> I mean, you could buy it, so it's definitely public.

Where ? official pineh64 site speaks only of modelB.

> 
> Model A also had HDMI, and it doesn't look like there's anything
> particularly specific with that board.

A subthread just say the opposite, modelA need something more for HDMI
https://lkml.org/lkml/2019/8/12/394

> 
> On the Ethernet side, the only thing that changes is the regulator /
> GPIO being used to enable the PHY?
> 

Yes

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ 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