Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 4/8] SPEAr13xx: Fixup: Move SPEAr1340 SATA platform code to system cfg driver
From: Pratyush Anand @ 2014-01-25  5:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201401242153.00848.arnd@arndb.de>

Hi Arnd,


On Sat, Jan 25, 2014 at 2:23 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Friday 24 January 2014, Pratyush Anand wrote:
> > On Thu, Jan 23, 2014 at 08:22:54PM +0800, Arnd Bergmann wrote:
> > > On Thursday 23 January 2014, Mohit Kumar wrote:
> > >
> > > I assume you'd want a phandle pointing to the syscon device in here
> > > as well?
> >
> > Since there is only one syscon device in the whole DT, so do I really
> > need to add phandle? Currently I am using
> > syscon_regmap_lookup_by_compatible to find  syscon device.
>
> I'd much rather use syscon_regmap_lookup_by_phandle than
> syscon_regmap_lookup_by_compatible, all the time, since this makes
> the relationship between the devices explicit.
>
> The phandle method also allows you to pass regmap indexes in the
> same property, which can be handy if two variants of the chip have
> the same registers at a different offset.
>
> > > > +/* SPEAr1340 Registers */
> > > > +/* Power Management Registers */

[...]

> > > Looking at the actual code now, this very much looks like it ought to
> > > be a "phy" driver and get put in drivers/phy/.
> >
> > Actually these registers are part of common system configurations
> > register space (called as misc space) for SPEAr SOC. So we opted for
> > syscon framework.
>
> The use of syscon for this is good, I have no objection to that, and
> was suggesting that you create a logical "phy" device that uses the
> misc syscon device as a backend.
>
> > PHY registers space starts from 0xEB800000, which can be
> > programmed for various phy specific functions like power management,
> > tx/rx settings, comparator settings etc. In most of the cases phy
> > works with default settings, however there are few exceptions for
> > which we will be adding a phy driver for further improvement of SPEAr
> > drivers.
>
> I see. So while the code you have here could be expressed as a phy driver
> by itself, there is another part of the SoC that controls the actual
> phy. How about if you add the phy device node to DT, and write a driver
> that doesn't actually program the phy registers for now, but does contain
> the code that you have posted here. That would give you flexibility for
> future extensions and at the same time let you remove all SPEAr specific
> code from the actual AHCI driver by using the generic ahci-platform
> driver.

OK..
--  will move all these code to a phy driver.
-- so, no need of a new cfg node as of now.
-- will pass syscon phandle to phy driver.
-- currently will keep ahci platform plugin (as it is here) in phy driver.
will remove when generic ahci driver is in place.

Regards
Pratyush
>
>         Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 0/2] DT updates for Hummingboard and new Cubox-i
From: Olof Johansson @ 2014-01-25  3:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140124150927.GA2780@S2101-09.ap.freescale.net>

Hi,

On Fri, Jan 24, 2014 at 7:09 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Sun, Jan 19, 2014 at 10:52:46AM -0800, Olof Johansson wrote:
>> I'm well aware that the DT branch has not been merged, and that's because
>> there has been no consensus from DT maintainers on the massive rework
>> you did as the base of the branch.
>
> I guess the 'massive rework' you're talking about is the patch series
> that begins with 'ARM: dts: imx6qdl: make pinctrl nodes board specific'?
> If so, I hope you would agree that we're trying to solve a real
> problem [1], even though the solution does not look like the best to
> you.
>
> So which specific part of the solution are you objecting to?  Those
> MX6QDL_*_PINGRP macros defined in imx6qdl-pingrp.h?  If that's the
> case, I can send a follow-up patch to kill the macros by filling in the
> pin group definitions directly.  The pros is that the pin group
> definitions for given board will be more intuitive, and the cons is that
> the change set will be even more massive, because the same multi-lines
> pin group definitions will be duplicated in multiple board dts files,
> which use the same group of pins for given device.
>
> Or any other better idea?

I haven't heard any other maintainers being positive to this change.
Not Linus W, as the pinctrl maintainer, and none of the DT
maintainers. I asked for their review, which should mean acks or
general approval from them, and I haven't seen any come in. Or did I
miss it?

>> Given the timing, it's not looking like it will be resolved in time for
>> the 3.14 merge window (which might open today).
>
> That's frustrating and will be surprising a lot of people who expect to
> see their board support in 3.14 release.

Yes, it is very unfortunate.

See this as a chance to adjust your process. Next time you send a
large pull request that gets questions like these, please consider
splitting off the controversial part in a separate branch so that the
bulk of the changes can go in. That's what other maintainers tend to
do. Right now you have chosen to let your pinctrl header file changes
hold up everything for everyone that merges code through you.


-Olof

^ permalink raw reply

* [PATCH] ARM: tegra: rebuild tegra_defconfig to add DEBUG_FS
From: Stephen Warren @ 2014-01-25  3:13 UTC (permalink / raw)
  To: linux-arm-kernel

From: Stephen Warren <swarren@nvidia.com>

DEBUG_FS used to be selected by COMMON_CLK_DEBUG, which was enabled
by tegra_defconfig. However, this config option no longer exists, so
no longer selects DEBUG_FS, and nothing else selects it either. So,
"make tegra_defconfig" no longer enables DEBUG_FS in .config.

Rebuild tegra_defconfig on top of next-20140424, while manually
re-enabling DEBUG_FS.

Reasons for removed entries are:
- I2C_MUX: selected by MEDIA_SUBDRV_AUTOSELECT
- DRM_PANEL: selected by DRM_TEGRA
- NEW_LEDS: selected by many things; at least VT
- LEDS_CLASS: selected by many things; at least VT
- LEDS_TRIGGERS: selected by many things; at least VT
- COMMON_CLK_DEBUG: no longer exists

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 arch/arm/configs/tegra_defconfig | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index 5fdc9a0..00fe9e9 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -29,11 +29,11 @@ CONFIG_ARCH_TEGRA_3x_SOC=y
 CONFIG_ARCH_TEGRA_114_SOC=y
 CONFIG_ARCH_TEGRA_124_SOC=y
 CONFIG_TEGRA_EMC_SCALING_ENABLE=y
+CONFIG_TRUSTED_FOUNDATIONS=y
 CONFIG_PCI=y
 CONFIG_PCI_MSI=y
 CONFIG_PCI_TEGRA=y
 CONFIG_PCIEPORTBUS=y
-CONFIG_TRUSTED_FOUNDATIONS=y
 CONFIG_SMP=y
 CONFIG_PREEMPT=y
 CONFIG_AEABI=y
@@ -125,7 +125,6 @@ CONFIG_SERIAL_TEGRA=y
 CONFIG_SERIAL_OF_PLATFORM=y
 # CONFIG_HW_RANDOM is not set
 # CONFIG_I2C_COMPAT is not set
-CONFIG_I2C_MUX=y
 CONFIG_I2C_MUX_PINCTRL=y
 CONFIG_I2C_TEGRA=y
 CONFIG_SPI=y
@@ -169,9 +168,8 @@ CONFIG_MEDIA_CAMERA_SUPPORT=y
 CONFIG_MEDIA_USB_SUPPORT=y
 CONFIG_USB_VIDEO_CLASS=m
 CONFIG_DRM=y
-CONFIG_DRM_PANEL=y
-CONFIG_DRM_PANEL_SIMPLE=y
 CONFIG_DRM_TEGRA=y
+CONFIG_DRM_PANEL_SIMPLE=y
 CONFIG_BACKLIGHT_LCD_SUPPORT=y
 # CONFIG_LCD_CLASS_DEVICE is not set
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
@@ -206,10 +204,7 @@ CONFIG_MMC_BLOCK_MINORS=16
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_PLTFM=y
 CONFIG_MMC_SDHCI_TEGRA=y
-CONFIG_NEW_LEDS=y
-CONFIG_LEDS_CLASS=y
 CONFIG_LEDS_GPIO=y
-CONFIG_LEDS_TRIGGERS=y
 CONFIG_LEDS_TRIGGER_TIMER=y
 CONFIG_LEDS_TRIGGER_ONESHOT=y
 CONFIG_LEDS_TRIGGER_HEARTBEAT=y
@@ -235,7 +230,6 @@ CONFIG_KEYBOARD_NVEC=y
 CONFIG_SERIO_NVEC_PS2=y
 CONFIG_NVEC_POWER=y
 CONFIG_NVEC_PAZ00=y
-CONFIG_COMMON_CLK_DEBUG=y
 CONFIG_TEGRA_IOMMU_GART=y
 CONFIG_TEGRA_IOMMU_SMMU=y
 CONFIG_MEMORY=y
@@ -265,6 +259,7 @@ CONFIG_NLS_CODEPAGE_437=y
 CONFIG_NLS_ISO8859_1=y
 CONFIG_PRINTK_TIME=y
 CONFIG_DEBUG_INFO=y
+CONFIG_DEBUG_FS=y
 CONFIG_MAGIC_SYSRQ=y
 CONFIG_DEBUG_SLAB=y
 CONFIG_DEBUG_VM=y
-- 
1.8.3.2

^ permalink raw reply related

* DT include files
From: Shawn Guo @ 2014-01-25  2:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2907925.DyxHkYtGr7@phil>

On Fri, Jan 24, 2014 at 09:02:09AM +0100, Heiko St?bner wrote:
> Hi Shawn,
> 
> did this topic get any final resolution - especially in terms of the pingrp.h 
> header?
> 
> As I'm currently preparing two board files (imx50 and imx6sl) this discussion 
> made me unsure if using the pin-group definitions is still the preferred style.

Yes, we need to get a conclusion ASAP.  The whole imx-dt-3.14 pull
request has been held for a few weeks because of that, and I'm asking
for direction there [1].

Shawn

[1] http://thread.gmane.org/gmane.linux.drivers.devicetree/58685/focus=296761

^ permalink raw reply

* [PATCH] clk: sunxi: fix overflow when setting up divided factors
From: Emilio López @ 2014-01-25  1:32 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, we are allocating space for two pointers, when we actually
may need to store three of them (two divisors plus the original clock).
Fix this, and change sizeof(type) to sizeof(*var) to keep checkpatch.pl
happy.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
---

Hi Mike,

Is there any chance you can include this oneliner on your second part of
the pull request to Linus?

Cheers,

Emilio

 drivers/clk/sunxi/clk-sunxi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 659e4ea..abb6c5a 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -875,7 +875,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
 	if (!clk_data)
 		return;
 
-	clks = kzalloc(SUNXI_DIVS_MAX_QTY * sizeof(struct clk *), GFP_KERNEL);
+	clks = kzalloc((SUNXI_DIVS_MAX_QTY+1) * sizeof(*clks), GFP_KERNEL);
 	if (!clks)
 		goto free_clkdata;
 
-- 
1.8.5.3

^ permalink raw reply related

* [PATCH v2] ARM: mm: Fix stage-2 device memory attributes
From: Christoffer Dall @ 2014-01-24 23:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1388852843-30986-1-git-send-email-christoffer.dall@linaro.org>

On Sat, Jan 04, 2014 at 08:27:23AM -0800, Christoffer Dall wrote:
> The stage-2 memory attributes are distinct from the Hyp memory
> attributes and the Stage-1 memory attributes.  We were using the stage-1
> memory attributes for stage-2 mappings causing device mappings to be
> mapped as normal memory.  Add the S2 equivalent defines for memory
> attributes and fix the comments explaining the defines while at it.
> 
> Add a prot_pte_s2 field to the mem_type struct and fill out the field
> for device mappings accordingly.
> 
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> Changelog[v2]:
>  - Guard the use of L_PTE_S2 defines with s2_policy to allow non-LPAE compiles.
> 
>  arch/arm/include/asm/pgtable-3level.h | 20 +++++++++++++-------
>  arch/arm/mm/mm.h                      |  1 +
>  arch/arm/mm/mmu.c                     | 15 ++++++++++++++-
>  3 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/include/asm/pgtable-3level.h b/arch/arm/include/asm/pgtable-3level.h
> index 4f95039..d5e04d6 100644
> --- a/arch/arm/include/asm/pgtable-3level.h
> +++ b/arch/arm/include/asm/pgtable-3level.h
> @@ -120,13 +120,19 @@
>  /*
>   * 2nd stage PTE definitions for LPAE.
>   */
> -#define L_PTE_S2_MT_UNCACHED	 (_AT(pteval_t, 0x5) << 2) /* MemAttr[3:0] */
> -#define L_PTE_S2_MT_WRITETHROUGH (_AT(pteval_t, 0xa) << 2) /* MemAttr[3:0] */
> -#define L_PTE_S2_MT_WRITEBACK	 (_AT(pteval_t, 0xf) << 2) /* MemAttr[3:0] */
> -#define L_PTE_S2_RDONLY		 (_AT(pteval_t, 1) << 6)   /* HAP[1]   */
> -#define L_PTE_S2_RDWR		 (_AT(pteval_t, 3) << 6)   /* HAP[2:1] */
> -
> -#define L_PMD_S2_RDWR		 (_AT(pmdval_t, 3) << 6)   /* HAP[2:1] */
> +#define L_PTE_S2_MT_UNCACHED		(_AT(pteval_t, 0x0) << 2) /* strongly ordered */
> +#define L_PTE_S2_MT_WRITETHROUGH	(_AT(pteval_t, 0xa) << 2) /* normal inner write-through */
> +#define L_PTE_S2_MT_WRITEBACK		(_AT(pteval_t, 0xf) << 2) /* normal inner write-back */
> +#define L_PTE_S2_MT_DEV_SHARED		(_AT(pteval_t, 0x1) << 2) /* device */
> +#define L_PTE_S2_MT_DEV_NONSHARED	(_AT(pteval_t, 0x1) << 2) /* device */
> +#define L_PTE_S2_MT_DEV_WC		(_AT(pteval_t, 0x5) << 2) /* normal non-cacheable */
> +#define L_PTE_S2_MT_DEV_CACHED		(_AT(pteval_t, 0xf) << 2) /* normal inner write-back */
> +#define L_PTE_S2_MT_MASK		(_AT(pteval_t, 0xf) << 2)
> +
> +#define L_PTE_S2_RDONLY			(_AT(pteval_t, 1) << 6)   /* HAP[1]   */
> +#define L_PTE_S2_RDWR			(_AT(pteval_t, 3) << 6)   /* HAP[2:1] */
> +
> +#define L_PMD_S2_RDWR			(_AT(pmdval_t, 3) << 6)   /* HAP[2:1] */
>  
>  /*
>   * Hyp-mode PL2 PTE definitions for LPAE.
> diff --git a/arch/arm/mm/mm.h b/arch/arm/mm/mm.h
> index d5a982d..7ea641b 100644
> --- a/arch/arm/mm/mm.h
> +++ b/arch/arm/mm/mm.h
> @@ -38,6 +38,7 @@ static inline pmd_t *pmd_off_k(unsigned long virt)
>  
>  struct mem_type {
>  	pteval_t prot_pte;
> +	pteval_t prot_pte_s2;
>  	pmdval_t prot_l1;
>  	pmdval_t prot_sect;
>  	unsigned int domain;
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 580ef2d..44d571f 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -231,36 +231,48 @@ __setup("noalign", noalign_setup);
>  #endif /* ifdef CONFIG_CPU_CP15 / else */
>  
>  #define PROT_PTE_DEVICE		L_PTE_PRESENT|L_PTE_YOUNG|L_PTE_DIRTY|L_PTE_XN
> +#define PROT_PTE_S2_DEVICE	PROT_PTE_DEVICE
>  #define PROT_SECT_DEVICE	PMD_TYPE_SECT|PMD_SECT_AP_WRITE
>  
>  static struct mem_type mem_types[] = {
>  	[MT_DEVICE] = {		  /* Strongly ordered / ARMv6 shared device */
>  		.prot_pte	= PROT_PTE_DEVICE | L_PTE_MT_DEV_SHARED |
>  				  L_PTE_SHARED,
> +		.prot_pte_s2	= s2_policy(PROT_PTE_S2_DEVICE) |
> +				  s2_policy(L_PTE_S2_MT_DEV_SHARED) |
> +				  L_PTE_SHARED,
>  		.prot_l1	= PMD_TYPE_TABLE,
>  		.prot_sect	= PROT_SECT_DEVICE | PMD_SECT_S,
>  		.domain		= DOMAIN_IO,
>  	},
>  	[MT_DEVICE_NONSHARED] = { /* ARMv6 non-shared device */
>  		.prot_pte	= PROT_PTE_DEVICE | L_PTE_MT_DEV_NONSHARED,
> +		.prot_pte_s2	= s2_policy(PROT_PTE_S2_DEVICE) |
> +				  s2_policy(L_PTE_S2_MT_DEV_NONSHARED),
>  		.prot_l1	= PMD_TYPE_TABLE,
>  		.prot_sect	= PROT_SECT_DEVICE,
>  		.domain		= DOMAIN_IO,
>  	},
>  	[MT_DEVICE_CACHED] = {	  /* ioremap_cached */
>  		.prot_pte	= PROT_PTE_DEVICE | L_PTE_MT_DEV_CACHED,
> +		.prot_pte_s2	= s2_policy(PROT_PTE_S2_DEVICE) |
> +				  s2_policy(L_PTE_S2_MT_DEV_CACHED),
>  		.prot_l1	= PMD_TYPE_TABLE,
>  		.prot_sect	= PROT_SECT_DEVICE | PMD_SECT_WB,
>  		.domain		= DOMAIN_IO,
>  	},
>  	[MT_DEVICE_WC] = {	/* ioremap_wc */
>  		.prot_pte	= PROT_PTE_DEVICE | L_PTE_MT_DEV_WC,
> +		.prot_pte_s2	= s2_policy(PROT_PTE_S2_DEVICE) |
> +				  s2_policy(L_PTE_S2_MT_DEV_WC),
>  		.prot_l1	= PMD_TYPE_TABLE,
>  		.prot_sect	= PROT_SECT_DEVICE,
>  		.domain		= DOMAIN_IO,
>  	},
>  	[MT_UNCACHED] = {
>  		.prot_pte	= PROT_PTE_DEVICE,
> +		.prot_pte_s2	= s2_policy(PROT_PTE_S2_DEVICE) |
> +				  s2_policy(L_PTE_S2_MT_UNCACHED),
>  		.prot_l1	= PMD_TYPE_TABLE,
>  		.prot_sect	= PMD_TYPE_SECT | PMD_SECT_XN,
>  		.domain		= DOMAIN_IO,
> @@ -458,7 +470,8 @@ static void __init build_mem_type_table(void)
>  	cp = &cache_policies[cachepolicy];
>  	vecs_pgprot = kern_pgprot = user_pgprot = cp->pte;
>  	s2_pgprot = cp->pte_s2;
> -	hyp_device_pgprot = s2_device_pgprot = mem_types[MT_DEVICE].prot_pte;
> +	hyp_device_pgprot = mem_types[MT_DEVICE].prot_pte;
> +	s2_device_pgprot = mem_types[MT_DEVICE].prot_pte_s2;
>  
>  	/*
>  	 * ARMv6 and above have extended page tables.
> -- 
> 1.8.5
> 

Ping?

-- 
Christoffer

^ permalink raw reply

* [PATCH V4 5/5] Documentation: power: reset: Add documentation for generic SYSCON reboot driver
From: Marc C @ 2014-01-24 23:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140124182355.GF4758@e106331-lin.cambridge.arm.com>

Hi Mark,

>> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> new file mode 100644
>> index 0000000..e9eb1fe
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>> @@ -0,0 +1,16 @@
>> +Generic SYSCON mapped register reset driver
>
> Bindings should describe hardware, not drivers

In a perfect world, the hardware designers would place _all_ of the registers needed to
support rebooting in a contiguous section of the memory map. However, this isn't the case
on some platforms, especially on ARM-based SoCs.

While I completely agree with you that the bindings describe hardware, I don't see how
Feng's work is contrary to that. Feng is working on logically-grouping an otherwise
"random" set of registers into a logical grouping. In this case, Feng is uniting a group
of registers and calling them the "reboot" register block.

> What's wrong with having a system clock unit binding, that the kernel
> can decompose as appropriate?

>From what I understand, the arm-soc maintainers want to reduce (and perhaps even
eliminate) these board-specific constructs, and try to utilize common driver-code that
resides in the "driver" folder. I can vouch for the syscon/regmap framework as something
which would enable the effort.

Thanks,
Marc C

On 01/24/2014 10:23 AM, Mark Rutland wrote:
> On Fri, Jan 24, 2014 at 06:03:10PM +0000, Feng Kan wrote:
>> On Fri, Jan 24, 2014 at 3:39 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>>> On Thu, Jan 23, 2014 at 07:20:01PM +0000, Feng Kan wrote:
>>>> Add documentation for generic SYSCON reboot driver.
>>>>
>>>> Signed-off-by: Feng Kan <fkan@apm.com>
>>>> ---
>>>>  .../bindings/power/reset/syscon-reboot.txt         |   16 ++++++++++++++++
>>>>  1 files changed, 16 insertions(+), 0 deletions(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>>>> new file mode 100644
>>>> index 0000000..e9eb1fe
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
>>>> @@ -0,0 +1,16 @@
>>>> +Generic SYSCON mapped register reset driver
>>>
>>> Bindings should describe hardware, not drivers.
>>>
>>> What precisely does this binding describe?
>>>
>>>> +
>>>> +Required properties:
>>>> +- compatible: should contain "syscon-reboot"
>>>> +- regmap: this is phandle to the register map node
>>>> +- offset: offset in the register map for the reboot register
>>>> +- mask: the reset value written to the reboot register
>>>> +
>>>> +Examples:
>>>> +
>>>> +reboot {
>>>> +   compatible = "syscon-reboot";
>>>> +   regmap = <&regmapnode>;
>>>> +   offset = <0x0>;
>>>> +   mask = <0x1>;
>>>> +};
>>>
>>> Access size? Endianness?
>> FKAN: are you asking for documentation? I don't see alot of example of
>> support for these.
> 
> If I used the enippet in the example, what endianness and access size
> should I expect an OS to perform? That should be documented.
> 
> If this doesn't match the general case, we can add properties later to
> adjust the access size and/or endianness. We just need to document what
> the binding actually describes currently, or it's not possible to
> implement anything based off of the binding documentation.
> 
> I should be able to read a binding document and write a dts. I shouldn't
> have to read the code to figure out what the binding describes.
> 
>>
>>>
>>> Why can we not have a binding for the register bank this exists in, and
>>> have that pass on the appropriate details to a syscon-reboot driver?
>>
>> FKAN: Thats a good idea. But the hardware in this case (SCU) system
>> clock unit has a bunch of registers used for different functions. If syscon is
>> used alot in this case and we pile more attribute into it. It would get kinda
>> messy after a while.
> 
> Huh?
> 
> What's wrong with having a system clock unit binding, that the kernel
> can decompose as appropriate?
> 
> I don't get your syscon argument.
> 
> Thanks,
> Mark.
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply

* [PATCHv2] arm64: Add CONFIG_CC_STACKPROTECTOR
From: Laura Abbott @ 2014-01-24 23:09 UTC (permalink / raw)
  To: linux-arm-kernel

arm64 currently lacks support for -fstack-protector. Add
similar functionality to arm to detect stack corruption.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
 arch/arm64/Kconfig                      |   11 +++++++++
 arch/arm64/Makefile                     |    4 +++
 arch/arm64/include/asm/stackprotector.h |   38 +++++++++++++++++++++++++++++++
 arch/arm64/kernel/process.c             |    6 +++++
 4 files changed, 59 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm64/include/asm/stackprotector.h

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 6d4dd22..47280cf 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -168,6 +168,17 @@ config HOTPLUG_CPU
 	  Say Y here to experiment with turning CPUs off and on.  CPUs
 	  can be controlled through /sys/devices/system/cpu.
 
+config CC_STACKPROTECTOR
+	bool "Enable -fstack-protector buffer overflow detection"
+	help
+	  This option turns on the -fstack-protector GCC feature. This
+	  feature puts, at the beginning of functions, a canary value on
+	  the stack just before the return address, and validates
+	  the value just before actually returning.  Stack based buffer
+	  overflows (that need to overwrite this return address) now also
+	  overwrite the canary, which gets detected and the attack is then
+	  neutralized via a kernel panic.
+
 source kernel/Kconfig.preempt
 
 config HZ
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 2fceb71..1ce221e 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -48,6 +48,10 @@ core-$(CONFIG_XEN) += arch/arm64/xen/
 libs-y		:= arch/arm64/lib/ $(libs-y)
 libs-y		+= $(LIBGCC)
 
+ifeq ($(CONFIG_CC_STACKPROTECTOR),y)
+KBUILD_CFLAGS	+=-fstack-protector
+endif
+
 # Default target when executing plain make
 KBUILD_IMAGE	:= Image.gz
 KBUILD_DTBS	:= dtbs
diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h
new file mode 100644
index 0000000..fe5e287
--- /dev/null
+++ b/arch/arm64/include/asm/stackprotector.h
@@ -0,0 +1,38 @@
+/*
+ * GCC stack protector support.
+ *
+ * Stack protector works by putting predefined pattern at the start of
+ * the stack frame and verifying that it hasn't been overwritten when
+ * returning from the function.  The pattern is called stack canary
+ * and gcc expects it to be defined by a global variable called
+ * "__stack_chk_guard" on ARM.  This unfortunately means that on SMP
+ * we cannot have a different canary value per task.
+ */
+
+#ifndef __ASM_STACKPROTECTOR_H
+#define __ASM_STACKPROTECTOR_H
+
+#include <linux/random.h>
+#include <linux/version.h>
+
+extern unsigned long __stack_chk_guard;
+
+/*
+ * Initialize the stackprotector canary value.
+ *
+ * NOTE: this must only be called from functions that never return,
+ * and it must always be inlined.
+ */
+static __always_inline void boot_init_stack_canary(void)
+{
+	unsigned long canary;
+
+	/* Try to get a semi random initial value. */
+	get_random_bytes(&canary, sizeof(canary));
+	canary ^= LINUX_VERSION_CODE;
+
+	current->stack_canary = canary;
+	__stack_chk_guard = current->stack_canary;
+}
+
+#endif	/* _ASM_STACKPROTECTOR_H */
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index de17c89..72c5188 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -50,6 +50,12 @@
 #include <asm/processor.h>
 #include <asm/stacktrace.h>
 
+#ifdef CONFIG_CC_STACKPROTECTOR
+#include <linux/stackprotector.h>
+unsigned long __stack_chk_guard __read_mostly;
+EXPORT_SYMBOL(__stack_chk_guard);
+#endif
+
 static void setup_restart(void)
 {
 	/*
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH] arm64: fix build error if DMA_CMA is enabled
From: Laura Abbott @ 2014-01-24 22:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140124153756.GH19052@arm.com>

On 1/24/2014 7:37 AM, Catalin Marinas wrote:
> On Fri, Jan 24, 2014 at 08:23:08AM +0000, Pankaj Dubey wrote:
>> arm64/include/asm/dma-contiguous.h is trying to include
>> <asm-genric/dma-contiguous.h> which does not exist, and thus failing
>> build for arm64 if we enable CONFIG_DMA_CMA. This patch fixes build
>> error by removing unwanted header inclusion from arm64's dma-contiguous.h.
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> Signed-off-by: Somraj Mani <somraj.mani@samsung.com>
>> ---
>>   arch/arm64/include/asm/dma-contiguous.h |    1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/dma-contiguous.h b/arch/arm64/include/asm/dma-contiguous.h
>> index d6aacb6..14c4c0c 100644
>> --- a/arch/arm64/include/asm/dma-contiguous.h
>> +++ b/arch/arm64/include/asm/dma-contiguous.h
>> @@ -18,7 +18,6 @@
>>   #ifdef CONFIG_DMA_CMA
>>
>>   #include <linux/types.h>
>> -#include <asm-generic/dma-contiguous.h>
>
> Thanks for this.
>
> Laura, did you have additional patches adding
> asm-generic/dma-contiguous.h?
>

no, asm-generic/dma-contiguous.h was an old file which was later 
removed. I missed this when rebasing from my older branch to mainline.
You can have

Acked-by: Laura Abbott <lauraa@codeaurora.org>

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply

* [PATCH] ARM-i.MX6Q-dts : Added USB_OTG Support
From: Sascha Hauer @ 2014-01-24 22:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390555724-9779-1-git-send-email-ashutosh.s@phytec.in>

Please add Phytec phyFLEX-i.MX6 to the subject. I assumed some SoC
specific changes here.

Sascha

On Fri, Jan 24, 2014 at 02:58:44PM +0530, Ashutosh singh wrote:
> This patch adds support for USB_OTG on Phytec phyFLEX-i.MX6 Quad module.
> 
> Signed-off-by: Ashutosh singh <ashutosh.s@phytec.in>
> ---
>  arch/arm/boot/dts/imx6q-phytec-pbab01.dts  |    4 ++++
>  arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi |   22 ++++++++++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6q-phytec-pbab01.dts b/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
> index 7d37ec6..39e69bd 100644
> --- a/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
> +++ b/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
> @@ -32,3 +32,7 @@
>  &usdhc3 {
>  	status = "okay";
>  };
> +
> +&usbotg {
> +	status = "okay";
> +};
> diff --git a/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi b/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi
> index 1a3b50d..dcb1d59 100644
> --- a/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi
> +++ b/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi
> @@ -18,6 +18,19 @@
>  	memory {
>  		reg = <0x10000000 0x80000000>;
>  	};
> +
> +	regulators {
> +		compatible = "simple-bus";
> +
> +		reg_usb_otg_vbus: usb_otg_vbus {
> +			compatible = "regulator-fixed";
> +			regulator-name = "usb_otg_vbus";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			gpio = <&gpio4 15 0>;
> +			enable-active-low;
> +		};
> +	};
>  };
>  
>  &ecspi3 {
> @@ -134,6 +147,7 @@
>  				MX6QDL_PAD_EIM_D23__GPIO3_IO23 0x80000000
>  				MX6QDL_PAD_DISP0_DAT3__GPIO4_IO24 0x80000000 /* SPI NOR chipselect */
>  				MX6QDL_PAD_DI0_PIN15__GPIO4_IO17  0x80000000 /* PMIC interrupt */
> +				MX6QDL_PAD_KEY_ROW4__GPIO4_IO15   0x80000000 /* USB_OTG_PWR_EN */
>  			>;
>  		};
>  	};
> @@ -178,3 +192,11 @@
>          wp-gpios = <&gpio1 29 0>;
>          status = "disabled";
>  };
> +
> +&usbotg {
> +	vbus-supply = <&reg_usb_otg_vbus>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usbotg_1>;
> +	disable-over-current;
> +	status = "disabled";
> +};
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* [PATCH 1/3] irqchip: orion: clear bridge cause register on init
From: Ezequiel Garcia @ 2014-01-24 21:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390516686-2224-2-git-send-email-sebastian.hesselbarth@gmail.com>

On Thu, Jan 23, 2014 at 11:38:04PM +0100, Sebastian Hesselbarth wrote:
> It is good practice to mask and clear pending irqs on init. We already
> mask all irqs, so also clear the bridge irq cause register.
> 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Cc: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org
> ---
>  drivers/irqchip/irq-orion.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-orion.c b/drivers/irqchip/irq-orion.c
> index e51d40031884..4137c3d15284 100644
> --- a/drivers/irqchip/irq-orion.c
> +++ b/drivers/irqchip/irq-orion.c
> @@ -180,8 +180,9 @@ static int __init orion_bridge_irq_init(struct device_node *np,
>  	gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit;
>  	gc->chip_types[0].chip.irq_unmask = irq_gc_mask_set_bit;
>  
> -	/* mask all interrupts */
> +	/* mask and clear all interrupts */
>  	writel(0, gc->reg_base + ORION_BRIDGE_IRQ_MASK);
> +	writel(0, gc->reg_base + ORION_BRIDGE_IRQ_CAUSE);
>  

This looks a bit bogus to me, now that we are clearing the cause upon
irq_startup(). Don't have a strong opinion, it's just that I fail to see
why we'd want or need this change...
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH v5 8/8] ARM: brcmstb: dts: add a reference DTS for Broadcom 7445
From: Marc C @ 2014-01-24 21:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140124110910.GC814@e106331-lin.cambridge.arm.com>

Hi Mark,

> As I commented on v3 [1], these are contiguous and can be described with
> a single entry:
>
> memory {
> 	device_type = "memory";
> 	reg = <0x0 0x00000000 0x0 0xc0000000>;
> };
>
> Is there any reason to have three entries?

Oopsies, sorry for missing that.

On BCM7445 and derivatives, there are 3 memory controllers. For each memory controller,
the first 1GB of physical DRAM is mapped to:

* 0x00_0000_0000
* 0x00_4000_0000
* 0x00_8000_0000

The memory controllers aren't interleaved. So, it's possible for the SoC to have a
discontiguous memory-mapping, where a designer chooses not to populate physical DRAM in
the middle.

The 'reg' property was broken-up to have each chunk of memory given a dedicated memblock.

All that said, if you like, I can rework the patch as you've suggested.

Thanks,
Marc C


On 01/24/2014 03:09 AM, Mark Rutland wrote:
> On Wed, Jan 22, 2014 at 03:30:52AM +0000, Marc Carino wrote:
>> Add a sample DTS which will allow bootup of a board populated
>> with the BCM7445 chip.
>>
>> Signed-off-by: Marc Carino <marc.ceeeee@gmail.com>
>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  arch/arm/boot/dts/bcm7445.dts |  111 +++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 111 insertions(+), 0 deletions(-)
>>  create mode 100644 arch/arm/boot/dts/bcm7445.dts
>>
>> diff --git a/arch/arm/boot/dts/bcm7445.dts b/arch/arm/boot/dts/bcm7445.dts
>> new file mode 100644
>> index 0000000..ffa3305
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/bcm7445.dts
>> @@ -0,0 +1,111 @@
>> +/dts-v1/;
>> +/include/ "skeleton.dtsi"
>> +
>> +/ {
>> +	#address-cells = <2>;
>> +	#size-cells = <2>;
>> +	model = "Broadcom STB (bcm7445)";
>> +	compatible = "brcm,bcm7445", "brcm,brcmstb";
>> +	interrupt-parent = <&gic>;
>> +
>> +	chosen {};
>> +
>> +	memory {
>> +		device_type = "memory";
>> +		reg = <0x00 0x00000000 0x00 0x40000000>,
>> +		      <0x00 0x40000000 0x00 0x40000000>,
>> +		      <0x00 0x80000000 0x00 0x40000000>;
>> +	};
> 
> As I commented on v3 [1], these are contiguous and can be described with
> a single entry:
> 
> memory {
> 	device_type = "memory";
> 	reg = <0x0 0x00000000 0x0 0xc0000000>;
> };
> 
> Is there any reason to have three entries?
> 
> Thanks,
> Mark.
> 
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-January/225899.html
> 

^ permalink raw reply

* [PATCH v5 6/8] ARM: brcmstb: add misc. DT bindings for brcmstb
From: Marc C @ 2014-01-24 21:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140124110300.GB814@e106331-lin.cambridge.arm.com>

Hi Mark,

>> +reboot
>> +-------
>> +Required properties
>> +
>> +    - compatible
>> +        The string property "brcm,brcmstb-reboot".
>> +
>> +    - syscon
>> +        A phandle / integer array that points to the syscon node which describes
>> +        the general system reset registers.
>> +            o a phandle to "sun_top_ctrl"
>> +            o offset to the "reset source enable" register
>> +            o offset to the "software master reset" register
>
> How variable are these values?

Very much so. Future chips will have different register maps. Because of this, Arnd
suggested that we use 'syscon' and 'regmap' to alleviate this maintenance burden.

>> +example:
>> +    smpboot {
>> +        compatible = "brcm,brcmstb-smpboot";
>> +        syscon-cpu = <&hif_cpubiuctrl 0x88 0x178>;
>> +        syscon-cont = <&hif_continuation>;
>> +    };
>
> This looks odd. This doesn't seem like a device, but rather a grouping
> of disparate devices used for a particular software purpose.
>
>> +
>> +example:
>> +    reboot {
>> +        compatible = "brcm,brcmstb-reboot";
>> +        syscon = <&sun_top_ctrl 0x304 0x308>;
>> +    };
>
> As with smpboot, this seems odd.

Sure. Our H/W designers unfortunately didn't put the boot and restart registers into a
logical grouping, or standard register interface. Instead, they're all over the place.
How do you suggest naming the nodes to indicate this?

Thanks,
Marc C

On 01/24/2014 03:03 AM, Mark Rutland wrote:
> On Wed, Jan 22, 2014 at 03:30:50AM +0000, Marc Carino wrote:
>> Document the bindings that the Broadcom STB platform needs
>> for proper bootup.
>>
>> Signed-off-by: Marc Carino <marc.ceeeee@gmail.com>
>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  .../devicetree/bindings/arm/brcm-brcmstb.txt       |   95 ++++++++++++++++++++
>>  1 files changed, 95 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt b/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
>> new file mode 100644
>> index 0000000..3c436cc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
>> @@ -0,0 +1,95 @@
>> +ARM Broadcom STB platforms Device Tree Bindings
>> +-----------------------------------------------
>> +Boards with Broadcom Brahma15 ARM-based BCMxxxx (generally BCM7xxx variants)
>> +SoC shall have the following DT organization:
>> +
>> +Required root node properties:
>> +    - compatible: "brcm,bcm<chip_id>", "brcm,brcmstb"
>> +
>> +example:
>> +/ {
>> +    #address-cells = <2>;
>> +    #size-cells = <2>;
>> +    model = "Broadcom STB (bcm7445)";
>> +    compatible = "brcm,bcm7445", "brcm,brcmstb";
>> +
>> +Further, syscon nodes that map platform-specific registers used for general
>> +system control is required:
>> +
>> +    - compatible: "brcm,bcm<chip_id>-sun-top-ctrl", "syscon"
>> +    - compatible: "brcm,bcm<chip_id>-hif-cpubiuctrl", "syscon"
>> +    - compatible: "brcm,bcm<chip_id>-hif-continuation", "syscon"
>> +
>> +example:
>> +    rdb {
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
>> +        compatible = "simple-bus";
>> +        ranges = <0 0x00 0xf0000000 0x1000000>;
>> +
>> +        sun_top_ctrl: syscon at 404000 {
>> +            compatible = "brcm,bcm7445-sun-top-ctrl", "syscon";
>> +            reg = <0x404000 0x51c>;
>> +        };
>> +
>> +        hif_cpubiuctrl: syscon at 3e2400 {
>> +            compatible = "brcm,bcm7445-hif-cpubiuctrl", "syscon";
>> +            reg = <0x3e2400 0x5b4>;
>> +        };
>> +
>> +        hif_continuation: syscon at 452000 {
>> +            compatible = "brcm,bcm7445-hif-continuation", "syscon";
>> +            reg = <0x452000 0x100>;
>> +        };
>> +    };
>> +
>> +Lastly, nodes that allow for support of SMP initialization and reboot are
>> +required:
>> +
>> +smpboot
>> +-------
>> +Required properties:
>> +
>> +    - compatible
>> +        The string "brcm,brcmstb-smpboot".
>> +
>> +    - syscon-cpu
>> +        A phandle / integer array property which lets the BSP know the location
>> +        of certain CPU power-on registers.
>> +
>> +        The layout of the property is as follows:
>> +            o a phandle to the "hif_cpubiuctrl" syscon node
>> +            o offset to the base CPU power zone register
>> +            o offset to the base CPU reset register
> 
> How variable are these values?
> 
>> +
>> +    - syscon-cont
>> +        A phandle pointing to the syscon node which describes the CPU boot
>> +        continuation registers.
>> +            o a phandle to the "hif_continuation" syscon node
>> +
>> +example:
>> +    smpboot {
>> +        compatible = "brcm,brcmstb-smpboot";
>> +        syscon-cpu = <&hif_cpubiuctrl 0x88 0x178>;
>> +        syscon-cont = <&hif_continuation>;
>> +    };
> 
> This looks odd. This doesn't seem like a device, but rather a grouping
> of disparate devices used for a particular software purpose.
> 
>> +
>> +reboot
>> +-------
>> +Required properties
>> +
>> +    - compatible
>> +        The string property "brcm,brcmstb-reboot".
>> +
>> +    - syscon
>> +        A phandle / integer array that points to the syscon node which describes
>> +        the general system reset registers.
>> +            o a phandle to "sun_top_ctrl"
>> +            o offset to the "reset source enable" register
>> +            o offset to the "software master reset" register
> 
> How variable are these values?
> 
>> +
>> +example:
>> +    reboot {
>> +        compatible = "brcm,brcmstb-reboot";
>> +        syscon = <&sun_top_ctrl 0x304 0x308>;
>> +    };
> 
> As with smpboot, this seems odd.
> 
> Thanks,
> Mark.
> 

^ permalink raw reply

* [RFC PATCH V3 4/4] MAINTAINERS: entry for APM X-Gene PCIe host driver
From: Tanmay Inamdar @ 2014-01-24 21:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390599168-13150-1-git-send-email-tinamdar@apm.com>

Add entry for AppliedMicro X-Gene PCIe host driver.

Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
---
 MAINTAINERS |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5c21402..721fec7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6539,6 +6539,13 @@ L:	linux-pci at vger.kernel.org
 S:	Maintained
 F:	drivers/pci/host/*designware*
 
+PCI DRIVER FOR APPLIEDMICRO XGENE
+M:	Tanmay Inamdar <tinamdar@apm.com>
+L:	linux-pci at vger.kernel.org
+L:	linux-arm-kernel at lists.infradead.org
+S:	Maintained
+F:	drivers/pci/host/pci-xgene.c
+
 PCMCIA SUBSYSTEM
 P:	Linux PCMCIA Team
 L:	linux-pcmcia at lists.infradead.org
-- 
1.7.9.5

^ permalink raw reply related

* [RFC PATCH V3 3/4] dt-bindings: pci: xgene pcie device tree bindings
From: Tanmay Inamdar @ 2014-01-24 21:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390599168-13150-1-git-send-email-tinamdar@apm.com>

This patch adds the bindings for X-Gene PCIe driver. The driver resides
under 'drivers/pci/host/pci-xgene.c' file.

Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
---
 .../devicetree/bindings/pci/xgene-pci.txt          |   52 ++++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/xgene-pci.txt

diff --git a/Documentation/devicetree/bindings/pci/xgene-pci.txt b/Documentation/devicetree/bindings/pci/xgene-pci.txt
new file mode 100644
index 0000000..60e4a54
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/xgene-pci.txt
@@ -0,0 +1,52 @@
+* AppliedMicro X-Gene PCIe interface
+
+Required properties:
+- device_type: set to "pci"
+- compatible: should contain "xgene,pcie" to identify the core.
+- reg: A list of physical base address and length for each set of controller
+       registers. Must contain an entry for each entry in the reg-names
+       property.
+- reg-names: Must include the following entries:
+  "csr": controller configuration registers.
+  "cfg": pcie configuration space registers.
+- #address-cells: set to <3>
+- #size-cells: set to <2>
+- ranges: ranges for the outbound memory, I/O regions.
+- dma-ranges: ranges for the inbound memory regions.
+- #interrupt-cells: set to <1>
+- interrupt-map-mask and interrupt-map: standard PCI properties
+	to define the mapping of the PCIe interface to interrupt
+	numbers.
+- clocks: from common clock binding: handle to pci clock.
+
+Optional properties:
+- status: Either "ok" or "disabled".
+
+Example:
+
+SoC specific DT Entry:
+	pcie0: pcie at 1f2b0000 {
+		status = "disabled";
+		device_type = "pci";
+		compatible = "apm,xgene-pcie";
+		#interrupt-cells = <1>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		reg = < 0x00 0x1f2b0000 0x0 0x00010000   /* Controller registers */
+			0xe0 0xd0000000 0x0 0x00200000>; /* PCI config space */
+		reg-names = "csr", "cfg";
+		ranges = <0x01000000 0x00 0x00000000 0xe0 0x00000000 0x00 0x00010000   /* io */
+			  0x02000000 0x00 0x10000000 0xe0 0x10000000 0x00 0x80000000>; /* mem */
+		dma-ranges = <0x42000000 0x40 0x00000000 0x40 0x00000000 0x40 0x00000000>;
+		interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+		interrupt-map = <0x0 0x0 0x0 0x1 &gic 0x0 0xc2 0x1
+				 0x0 0x0 0x0 0x2 &gic 0x0 0xc3 0x1
+				 0x0 0x0 0x0 0x3 &gic 0x0 0xc4 0x1
+				 0x0 0x0 0x0 0x4 &gic 0x0 0xc5 0x1>;
+		clocks = <&pcie0clk 0>;
+	};
+
+Board specific DT Entry:
+	&pcie0 {
+		status = "ok";
+	};
-- 
1.7.9.5

^ permalink raw reply related

* [RFC PATCH V3 2/4] arm64: dts: APM X-Gene PCIe device tree nodes
From: Tanmay Inamdar @ 2014-01-24 21:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390599168-13150-1-git-send-email-tinamdar@apm.com>

This patch adds the device tree nodes for APM X-Gene PCIe controller and
PCIe clock interface. Since X-Gene SOC supports maximum 5 ports, 5 dts
nodes are added.

Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
---
 arch/arm64/boot/dts/apm-mustang.dts |    8 ++
 arch/arm64/boot/dts/apm-storm.dtsi  |  155 +++++++++++++++++++++++++++++++++++
 2 files changed, 163 insertions(+)

diff --git a/arch/arm64/boot/dts/apm-mustang.dts b/arch/arm64/boot/dts/apm-mustang.dts
index 1247ca1..507b6c9 100644
--- a/arch/arm64/boot/dts/apm-mustang.dts
+++ b/arch/arm64/boot/dts/apm-mustang.dts
@@ -24,3 +24,11 @@
 		reg = < 0x1 0x00000000 0x0 0x80000000 >; /* Updated by bootloader */
 	};
 };
+
+&pcie0clk {
+	status = "ok";
+};
+
+&pcie0 {
+	status = "ok";
+};
diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi
index d37d736..e579a6f 100644
--- a/arch/arm64/boot/dts/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm-storm.dtsi
@@ -176,6 +176,161 @@
 				reg-names = "csr-reg";
 				clock-output-names = "eth8clk";
 			};
+
+			pcie0clk: pcie0clk at 1f2bc000 {
+				status = "disabled";
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				reg = <0x0 0x1f2bc000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "pcie0clk";
+			};
+
+			pcie1clk: pcie1clk at 1f2cc000 {
+				status = "disabled";
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				reg = <0x0 0x1f2cc000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "pcie1clk";
+			};
+
+			pcie2clk: pcie2clk at 1f2dc000 {
+				status = "disabled";
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				reg = <0x0 0x1f2dc000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "pcie2clk";
+			};
+
+			pcie3clk: pcie3clk at 1f50c000 {
+				status = "disabled";
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				reg = <0x0 0x1f50c000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "pcie3clk";
+			};
+
+			pcie4clk: pcie4clk at 1f51c000 {
+				status = "disabled";
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				reg = <0x0 0x1f51c000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "pcie4clk";
+			};
+		};
+
+		pcie0: pcie at 1f2b0000 {
+			status = "disabled";
+			device_type = "pci";
+			compatible = "apm,xgene-pcie";
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg = < 0x00 0x1f2b0000 0x0 0x00010000   /* Controller registers */
+				0xe0 0xd0000000 0x0 0x00200000>; /* PCI config space */
+			reg-names = "csr", "cfg";
+			ranges = <0x01000000 0x00 0x00000000 0xe0 0x00000000 0x00 0x00010000   /* io */
+				  0x02000000 0x00 0x10000000 0xe0 0x10000000 0x00 0x80000000>; /* mem */
+			dma-ranges = <0x42000000 0x40 0x00000000 0x40 0x00000000 0x40 0x00000000>;
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0x0 0x0 0x0 0x1 &gic 0x0 0xc2 0x1
+					 0x0 0x0 0x0 0x2 &gic 0x0 0xc3 0x1
+					 0x0 0x0 0x0 0x3 &gic 0x0 0xc4 0x1
+					 0x0 0x0 0x0 0x4 &gic 0x0 0xc5 0x1>;
+			clocks = <&pcie0clk 0>;
+		};
+
+		pcie1: pcie at 1f2c0000 {
+			status = "disabled";
+			device_type = "pci";
+			compatible = "apm,xgene-pcie";
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg = < 0x00 0x1f2c0000 0x0 0x00010000   /* Controller registers */
+				0xd0 0xd0000000 0x0 0x00200000>; /* PCI config space */
+			reg-names = "csr", "cfg";
+			ranges = <0x01000000 0x0 0x00000000 0xd0 0x00000000 0x00 0x00010000   /* io  */
+				  0x02000000 0x0 0x10000000 0xd0 0x10000000 0x00 0x80000000>; /* mem */
+			dma-ranges = <0x42000000 0x40 0x00000000 0x40 0x00000000 0x40 0x00000000>;
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0x0 0x0 0x0 0x1 &gic 0x0 0xc8 0x1
+					 0x0 0x0 0x0 0x2 &gic 0x0 0xc9 0x1
+					 0x0 0x0 0x0 0x3 &gic 0x0 0xca 0x1
+					 0x0 0x0 0x0 0x4 &gic 0x0 0xcb 0x1>;
+			clocks = <&pcie1clk 0>;
+		};
+
+		pcie2: pcie at 1f2d0000 {
+			status = "disabled";
+			device_type = "pci";
+			compatible = "apm,xgene-pcie";
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg =  < 0x00 0x1f2d0000 0x0 0x00010000   /* Controller registers */
+				 0x90 0xd0000000 0x0 0x00200000>; /* PCI config space */
+			reg-names = "csr", "cfg";
+			ranges = <0x01000000 0x0 0x00000000 0x90 0x00000000 0x0 0x00010000   /* io  */
+				  0x02000000 0x0 0x10000000 0x90 0x10000000 0x0 0x80000000>; /* mem */
+			dma-ranges = <0x42000000 0x40 0x00000000 0x40 0x00000000 0x40 0x00000000>;
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0x0 0x0 0x0 0x1 &gic 0x0 0xce 0x1
+					 0x0 0x0 0x0 0x2 &gic 0x0 0xcf 0x1
+					 0x0 0x0 0x0 0x3 &gic 0x0 0xd0 0x1
+					 0x0 0x0 0x0 0x4 &gic 0x0 0xd1 0x1>;
+			clocks = <&pcie2clk 0>;
+		};
+
+		pcie3: pcie at 1f500000 {
+			status = "disabled";
+			device_type = "pci";
+			compatible = "apm,xgene-pcie";
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg = < 0x00 0x1f500000 0x0 0x00010000   /* Controller registers */
+				0xa0 0xd0000000 0x0 0x00200000>; /* PCI config space */
+			reg-names = "csr", "cfg";
+			ranges = <0x01000000 0x0 0x00000000 0xa0 0x00000000 0x0 0x00010000  /* mem */
+				  0x02000000 0x0 0x10000000 0xa0 0x10000000 0x0 0x80000000>; /* io  */
+			dma-ranges = <0x42000000 0x40 0x00000000 0x40 0x00000000 0x40 0x00000000>;
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0x0 0x0 0x0 0x1 &gic 0x0 0xd4 0x1
+					 0x0 0x0 0x0 0x2 &gic 0x0 0xd5 0x1
+					 0x0 0x0 0x0 0x3 &gic 0x0 0xd6 0x1
+					 0x0 0x0 0x0 0x4 &gic 0x0 0xd7 0x1>;
+			clocks = <&pcie3clk 0>;
+		};
+
+		pcie4: pcie at 1f510000 {
+			status = "disabled";
+			device_type = "pci";
+			compatible = "apm,xgene-pcie";
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg = < 0x00 0x1f510000 0x0 0x00010000   /* Controller registers */
+				0xc0 0xd0000000 0x0 0x00200000>; /* PCI config space */
+			reg-names = "csr", "cfg";
+			ranges = <0x01000000 0x0 0x00000000 0xc0 0x00000000 0x0 0x00010000   /* io  */
+				  0x02000000 0x0 0x10000000 0xc0 0x10000000 0x0 0x80000000>; /* mem */
+			dma-ranges = <0x42000000 0x40 0x00000000 0x40 0x00000000 0x40 0x00000000>;
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0x0 0x0 0x0 0x1 &gic 0x0 0xda 0x1
+					 0x0 0x0 0x0 0x2 &gic 0x0 0xdb 0x1
+					 0x0 0x0 0x0 0x3 &gic 0x0 0xdc 0x1
+					 0x0 0x0 0x0 0x4 &gic 0x0 0xdd 0x1>;
+			clocks = <&pcie4clk 0>;
 		};
 
 		serial0: serial at 1c020000 {
-- 
1.7.9.5

^ permalink raw reply related

* [RFC PATCH V3 1/4] pci: APM X-Gene PCIe controller driver
From: Tanmay Inamdar @ 2014-01-24 21:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390599168-13150-1-git-send-email-tinamdar@apm.com>

This patch adds the AppliedMicro X-Gene SOC PCIe controller driver.
X-Gene PCIe controller supports maximum upto 8 lanes and GEN3 speed.
X-Gene has maximum 5 PCIe ports supported.

Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
---
 drivers/pci/host/Kconfig     |   10 +
 drivers/pci/host/Makefile    |    1 +
 drivers/pci/host/pci-xgene.c |  784 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 795 insertions(+)
 create mode 100644 drivers/pci/host/pci-xgene.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 47d46c6..19ce97d 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -33,4 +33,14 @@ config PCI_RCAR_GEN2
 	  There are 3 internal PCI controllers available with a single
 	  built-in EHCI/OHCI host controller present on each one.
 
+config PCI_XGENE
+	bool "X-Gene PCIe controller"
+	depends on ARCH_XGENE
+	depends on OF
+	select PCIEPORTBUS
+	help
+	  Say Y here if you want internal PCI support on APM X-Gene SoC.
+	  There are 5 internal PCIe ports available. Each port is GEN3 capable
+	  and have varied lanes from x1 to x8.
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 13fb333..34c7c36 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
 obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
 obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
 obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
+obj-$(CONFIG_PCI_XGENE) += pci-xgene.o
diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
new file mode 100644
index 0000000..650a860
--- /dev/null
+++ b/drivers/pci/host/pci-xgene.c
@@ -0,0 +1,784 @@
+/**
+ * APM X-Gene PCIe Driver
+ *
+ * Copyright (c) 2013 Applied Micro Circuits Corporation.
+ *
+ * Author: Tanmay Inamdar <tinamdar@apm.com>.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+#include <linux/clk-private.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/jiffies.h>
+#include <linux/memblock.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_pci.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <asm/pcibios.h>
+
+#define PCIECORE_LTSSM			0x4c
+#define PCIECORE_CTLANDSTATUS		0x50
+#define PIPE_PHY_RATE_RD(src)		((0xc000 & (u32)(src)) >> 0xe)
+#define INTXSTATUSMASK			0x6c
+#define PIM1_1L				0x80
+#define IBAR2				0x98
+#define IR2MSK				0x9c
+#define PIM2_1L				0xa0
+#define IBAR3L				0xb4
+#define IR3MSKL				0xbc
+#define PIM3_1L				0xc4
+#define OMR1BARL			0x100
+#define OMR2BARL			0x118
+#define CFGBARL				0x154
+#define CFGBARH				0x158
+#define CFGCTL				0x15c
+#define RTDID				0x160
+#define BRIDGE_CFG_0			0x2000
+#define BRIDGE_CFG_1			0x2004
+#define BRIDGE_CFG_4			0x2010
+#define BRIDGE_CFG_32			0x2030
+#define BRIDGE_CFG_14			0x2038
+#define BRIDGE_CTRL_1			0x2204
+#define BRIDGE_CTRL_2			0x2208
+#define BRIDGE_CTRL_5			0x2214
+#define BRIDGE_STATUS_0			0x2600
+#define MEM_RAM_SHUTDOWN                0xd070
+#define BLOCK_MEM_RDY                   0xd074
+
+#define DEVICE_PORT_TYPE_MASK		0x03c00000
+#define PM_FORCE_RP_MODE_MASK		0x00000400
+#define SWITCH_PORT_MODE_MASK		0x00000800
+#define CLASS_CODE_MASK			0xffffff00
+#define LINK_UP_MASK			0x00000100
+#define AER_OPTIONAL_ERROR_EN		0xffc00000
+#define XGENE_PCIE_DEV_CTRL		0x2f0f
+#define AXI_EP_CFG_ACCESS		0x10000
+#define ENABLE_ASPM			0x08000000
+#define XGENE_PORT_TYPE_RC		0x05000000
+#define BLOCK_MEM_RDY_VAL               0xFFFFFFFF
+#define EN_COHERENCY			0xF0000000
+#define EN_REG				0x00000001
+#define OB_LO_IO			0x00000002
+#define XGENE_PCIE_VENDORID		0xE008
+#define XGENE_PCIE_DEVICEID		0xE004
+#define XGENE_PCIE_TIMEOUT		(500*1000) /* us */
+#define XGENE_LTSSM_DETECT_WAIT		20
+#define XGENE_LTSSM_L0_WAIT		4
+#define SZ_1T				(SZ_1G*1024ULL)
+
+struct xgene_res_cfg {
+	struct resource		res;
+	u64			pci_addr;
+};
+
+struct xgene_pcie_port {
+	struct device_node	*node;
+	struct device		*dev;
+	struct clk		*clk;
+	struct xgene_res_cfg	mem;
+	struct xgene_res_cfg	io;
+	struct resource		realio;
+	void __iomem		*csr_base;
+	void __iomem		*cfg_base;
+	u8			link_up;
+};
+
+static inline u32 pcie_bar_low_val(u32 addr, u32 flags)
+{
+	return (addr & PCI_BASE_ADDRESS_MEM_MASK) | flags;
+}
+
+static inline struct xgene_pcie_port *
+xgene_pcie_bus_to_port(struct pci_bus *bus)
+{
+	struct pci_sys_data *sys = bus->sysdata;
+	return sys->private_data;
+}
+
+/* PCIE Configuration Out/In */
+static inline void xgene_pcie_cfg_out32(void __iomem *addr, u32 val)
+{
+	writel(val, addr);
+}
+
+static inline void xgene_pcie_cfg_out16(void __iomem *addr, u16 val)
+{
+	u64 temp_addr = (u64)addr & ~0x3;
+	u32 val32 = readl((void __iomem *)temp_addr);
+
+	switch ((u64) addr & 0x3) {
+	case 2:
+		val32 &= ~0xFFFF0000;
+		val32 |= (u32) val << 16;
+		break;
+	case 0:
+	default:
+		val32 &= ~0xFFFF;
+		val32 |= val;
+		break;
+	}
+	writel(val32, (void __iomem *)temp_addr);
+}
+
+static inline void xgene_pcie_cfg_out8(void __iomem *addr, u8 val)
+{
+	phys_addr_t temp_addr = (u64) addr & ~0x3;
+	u32 val32 = readl((void __iomem *)temp_addr);
+
+	switch ((u64) addr & 0x3) {
+	case 0:
+		val32 &= ~0xFF;
+		val32 |= val;
+		break;
+	case 1:
+		val32 &= ~0xFF00;
+		val32 |= (u32) val << 8;
+		break;
+	case 2:
+		val32 &= ~0xFF0000;
+		val32 |= (u32) val << 16;
+		break;
+	case 3:
+	default:
+		val32 &= ~0xFF000000;
+		val32 |= (u32) val << 24;
+		break;
+	}
+	writel(val32, (void __iomem *)temp_addr);
+}
+
+static inline void xgene_pcie_cfg_in32(void __iomem *addr, u32 *val)
+{
+	*val = readl(addr);
+}
+
+static inline void xgene_pcie_cfg_in16(void __iomem *addr, u16 *val)
+{
+	u64 temp_addr = (u64)addr & ~0x3;
+	u32 val32;
+
+	val32 = readl((void __iomem *)temp_addr);
+
+	switch ((u64)addr & 0x3) {
+	case 2:
+		*val = val32 >> 16;
+		break;
+	case 0:
+	default:
+		*val = val32;
+		break;
+	}
+}
+
+static inline void xgene_pcie_cfg_in8(void __iomem *addr, u8 *val)
+{
+	u64 temp_addr = (u64)addr & ~0x3;
+	u32 val32;
+
+	val32 = readl((void __iomem *)temp_addr);
+
+	switch ((u64)addr & 0x3) {
+	case 3:
+		*val = val32 >> 24;
+		break;
+	case 2:
+		*val = val32 >> 16;
+		break;
+	case 1:
+		*val = val32 >> 8;
+		break;
+	case 0:
+	default:
+		*val = val32;
+		break;
+	}
+}
+
+/* When the address bit [17:16] is 2'b01, the Configuration access will be
+ * treated as Type 1 and it will be forwarded to external PCIe device.
+ */
+static void __iomem *xgene_pcie_get_cfg_base(struct pci_bus *bus)
+{
+	struct xgene_pcie_port *port = xgene_pcie_bus_to_port(bus);
+
+	if (bus->number >= (bus->primary + 1))
+		return port->cfg_base + AXI_EP_CFG_ACCESS;
+
+	return port->cfg_base;
+}
+
+/* For Configuration request, RTDID register is used as Bus Number,
+ * Device Number and Function number of the header fields.
+ */
+static void xgene_pcie_set_rtdid_reg(struct pci_bus *bus, uint devfn)
+{
+	struct xgene_pcie_port *port = xgene_pcie_bus_to_port(bus);
+	unsigned int b, d, f;
+	u32 rtdid_val = 0;
+
+	b = bus->number;
+	d = PCI_SLOT(devfn);
+	f = PCI_FUNC(devfn);
+
+	if (!pci_is_root_bus(bus))
+		rtdid_val = (b << 8) | (d << 3) | f;
+
+	writel(rtdid_val, port->csr_base + RTDID);
+	/* read the register back to ensure flush */
+	readl(port->csr_base + RTDID);
+}
+
+static int xgene_pcie_read_config(struct pci_bus *bus, unsigned int devfn,
+				  int offset, int len, u32 *val)
+{
+	struct xgene_pcie_port *port = xgene_pcie_bus_to_port(bus);
+	void __iomem *addr;
+	u8 val8;
+	u16 val16;
+
+	if ((pci_is_root_bus(bus) && devfn != 0) || !port->link_up)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	xgene_pcie_set_rtdid_reg(bus, devfn);
+	addr = xgene_pcie_get_cfg_base(bus);
+	switch (len) {
+	case 1:
+		xgene_pcie_cfg_in8(addr + offset, &val8);
+		*val = val8;
+		break;
+	case 2:
+		xgene_pcie_cfg_in16(addr + offset, &val16);
+		*val = val16;
+		break;
+	default:
+		xgene_pcie_cfg_in32(addr + offset, val);
+		break;
+	}
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int xgene_pcie_write_config(struct pci_bus *bus, unsigned int devfn,
+				   int offset, int len, u32 val)
+{
+	struct xgene_pcie_port *port = xgene_pcie_bus_to_port(bus);
+	void __iomem *addr;
+
+	if ((pci_is_root_bus(bus) && devfn != 0) || !port->link_up)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	xgene_pcie_set_rtdid_reg(bus, devfn);
+	addr = xgene_pcie_get_cfg_base(bus);
+	switch (len) {
+	case 1:
+		xgene_pcie_cfg_out8(addr + offset, (u8) val);
+		break;
+	case 2:
+		xgene_pcie_cfg_out16(addr + offset, (u16) val);
+		break;
+	default:
+		xgene_pcie_cfg_out32(addr + offset, val);
+		break;
+	}
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static struct pci_ops xgene_pcie_ops = {
+	.read = xgene_pcie_read_config,
+	.write = xgene_pcie_write_config
+};
+
+static void xgene_pcie_program_core(void __iomem *csr_base)
+{
+	u32 val;
+
+	val = readl(csr_base + BRIDGE_CFG_0);
+	val |= AER_OPTIONAL_ERROR_EN;
+	writel(val, csr_base + BRIDGE_CFG_0);
+	writel(0x0, csr_base + INTXSTATUSMASK);
+	val = readl(csr_base + BRIDGE_CTRL_1);
+	val = (val & ~0xffff) | XGENE_PCIE_DEV_CTRL;
+	writel(val, csr_base + BRIDGE_CTRL_1);
+}
+
+static u64 xgene_pcie_set_ib_mask(void __iomem *csr_base, u32 addr,
+				  u32 flags, u64 size)
+{
+	u64 mask = (~(size - 1) & PCI_BASE_ADDRESS_MEM_MASK) | flags;
+	u32 val32 = 0;
+	u32 val;
+
+	val32 = readl(csr_base + addr);
+	val = (val32 & 0x0000ffff) | (lower_32_bits(mask) << 16);
+	writel(val, csr_base + addr);
+
+	val32 = readl(csr_base + addr + 0x04);
+	val = (val32 & 0xffff0000) | (lower_32_bits(mask) >> 16);
+	writel(val, csr_base + addr + 0x04);
+
+	val32 = readl(csr_base + addr + 0x04);
+	val = (val32 & 0x0000ffff) | (upper_32_bits(mask) << 16);
+	writel(val, csr_base + addr + 0x04);
+
+	val32 = readl(csr_base + addr + 0x08);
+	val = (val32 & 0xffff0000) | (upper_32_bits(mask) >> 16);
+	writel(val, csr_base + addr + 0x08);
+
+	return mask;
+}
+
+static void xgene_pcie_poll_linkup(struct xgene_pcie_port *port,
+				   u32 *lanes, u32 *speed)
+{
+	void __iomem *csr_base = port->csr_base;
+	u32 val32;
+	u64 start_time, time;
+
+	/*
+	 * A component enters the LTSSM Detect state within
+	 * 20ms of the end of fundamental core reset.
+	 */
+	msleep(XGENE_LTSSM_DETECT_WAIT);
+	port->link_up = 0;
+	start_time = jiffies;
+	do {
+		val32 = readl(csr_base + PCIECORE_CTLANDSTATUS);
+		if (val32 & LINK_UP_MASK) {
+			port->link_up = 1;
+			*speed = PIPE_PHY_RATE_RD(val32);
+			val32 = readl(csr_base + BRIDGE_STATUS_0);
+			*lanes = val32 >> 26;
+		}
+		time = jiffies_to_msecs(jiffies - start_time);
+	} while ((!port->link_up) || (time <= XGENE_LTSSM_L0_WAIT));
+}
+
+static void xgene_pcie_setup_root_complex(struct xgene_pcie_port *port)
+{
+	void __iomem *csr_base = port->csr_base;
+	u32 val;
+
+	val = (XGENE_PCIE_DEVICEID << 16) | XGENE_PCIE_VENDORID;
+	writel(val, csr_base + BRIDGE_CFG_0);
+
+	val = readl(csr_base + BRIDGE_CFG_1);
+	val &= ~CLASS_CODE_MASK;
+	val |= PCI_CLASS_BRIDGE_PCI << 16;
+	writel(val, csr_base + BRIDGE_CFG_1);
+
+	val = readl(csr_base + BRIDGE_CFG_14);
+	val |= SWITCH_PORT_MODE_MASK;
+	val &= ~PM_FORCE_RP_MODE_MASK;
+	writel(val, csr_base + BRIDGE_CFG_14);
+
+	val = readl(csr_base + BRIDGE_CTRL_5);
+	val &= ~DEVICE_PORT_TYPE_MASK;
+	val |= XGENE_PORT_TYPE_RC;
+	writel(val, csr_base + BRIDGE_CTRL_5);
+
+	val = readl(csr_base + BRIDGE_CTRL_2);
+	val |= ENABLE_ASPM;
+	writel(val, csr_base + BRIDGE_CTRL_2);
+
+	val = readl(csr_base + BRIDGE_CFG_32);
+	writel(val | (1 << 19), csr_base + BRIDGE_CFG_32);
+}
+
+/* Return 0 on success */
+static int xgene_pcie_init_ecc(struct xgene_pcie_port *port)
+{
+	void __iomem *csr_base = port->csr_base;
+	int timeout = XGENE_PCIE_TIMEOUT;
+	u32 val;
+
+	val = readl(csr_base + MEM_RAM_SHUTDOWN);
+	if (val == 0)
+		return 0;
+	writel(0x0, csr_base + MEM_RAM_SHUTDOWN);
+	do {
+		val = readl(csr_base + BLOCK_MEM_RDY);
+		udelay(1);
+	} while ((val != BLOCK_MEM_RDY_VAL) && timeout--);
+
+	return !(timeout > 0);
+}
+
+static int xgene_pcie_init_port(struct xgene_pcie_port *port)
+{
+	int rc;
+
+	port->clk = clk_get(port->dev, NULL);
+	if (IS_ERR_OR_NULL(port->clk)) {
+		dev_err(port->dev, "clock not available\n");
+		return -ENODEV;
+	}
+
+	rc = clk_prepare_enable(port->clk);
+	if (rc) {
+		dev_err(port->dev, "clock enable failed\n");
+		return rc;
+	}
+
+	rc = xgene_pcie_init_ecc(port);
+	if (rc) {
+		dev_err(port->dev, "memory init failed\n");
+		return rc;
+	}
+
+	return 0;
+}
+
+struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus)
+{
+	struct xgene_pcie_port *port = xgene_pcie_bus_to_port(bus);
+
+	return of_node_get(port->node);
+}
+
+static void xgene_pcie_fixup_bridge(struct pci_dev *dev)
+{
+	int i;
+
+	/* Hide the PCI host BARs from the kernel as their content doesn't
+	 * fit well in the resource management
+	 */
+	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
+		dev->resource[i].start = dev->resource[i].end = 0;
+		dev->resource[i].flags = 0;
+	}
+	dev_info(&dev->dev, "Hiding X-Gene pci host bridge resources %s\n",
+		 pci_name(dev));
+}
+DECLARE_PCI_FIXUP_HEADER(XGENE_PCIE_VENDORID, XGENE_PCIE_DEVICEID,
+			 xgene_pcie_fixup_bridge);
+
+static int xgene_pcie_map_reg(struct xgene_pcie_port *port,
+			      struct platform_device *pdev, u64 *cfg_addr)
+{
+	struct resource *res;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "csr");
+	port->csr_base = devm_ioremap_resource(port->dev, res);
+	if (IS_ERR(port->csr_base))
+		return PTR_ERR(port->csr_base);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
+	port->cfg_base = devm_ioremap_resource(port->dev, res);
+	if (IS_ERR(port->cfg_base))
+		return PTR_ERR(port->cfg_base);
+	*cfg_addr = res->start;
+
+	return 0;
+}
+
+static void xgene_pcie_setup_ob_reg(struct xgene_pcie_port *port,
+				    u32 addr, u32 restype)
+{
+	struct resource *res;
+	void __iomem *base = port->csr_base + addr;
+	resource_size_t size;
+	u64 cpu_addr, pci_addr;
+	u64 mask = 0;
+	u32 min_size;
+	u32 flag = EN_REG;
+
+	if (restype == IORESOURCE_MEM) {
+		res = &port->mem.res;
+		pci_addr = port->mem.pci_addr;
+		min_size = SZ_128M;
+	} else {
+		res = &port->io.res;
+		pci_addr = port->io.pci_addr;
+		min_size = 128;
+		flag |= OB_LO_IO;
+	}
+	size = resource_size(res);
+	if (size >= min_size)
+		mask = ~(size - 1) | flag;
+	else
+		dev_warn(port->dev, "res size 0x%llx less than minimum 0x%x\n",
+			 (u64)size, min_size);
+	cpu_addr = res->start;
+	writel(lower_32_bits(cpu_addr), base);
+	writel(upper_32_bits(cpu_addr), base + 0x04);
+	writel(lower_32_bits(mask), base + 0x08);
+	writel(upper_32_bits(mask), base + 0x0c);
+	writel(lower_32_bits(pci_addr), base + 0x10);
+	writel(upper_32_bits(pci_addr), base + 0x14);
+}
+
+static void xgene_pcie_setup_cfg_reg(void __iomem *csr_base, u64 addr)
+{
+	writel(lower_32_bits(addr), csr_base + CFGBARL);
+	writel(upper_32_bits(addr), csr_base + CFGBARH);
+	writel(EN_REG, csr_base + CFGCTL);
+}
+
+static int xgene_pcie_parse_map_ranges(struct xgene_pcie_port *port,
+				       u64 cfg_addr)
+{
+	struct device_node *np = port->node;
+	struct of_pci_range range;
+	struct of_pci_range_parser parser;
+	struct device *dev = port->dev;
+
+	if (of_pci_range_parser_init(&parser, np)) {
+		dev_err(dev, "missing ranges property\n");
+		return -EINVAL;
+	}
+
+	/* Get the I/O, memory ranges from DT */
+	for_each_of_pci_range(&parser, &range) {
+		struct resource *res = NULL;
+		u64 restype = range.flags & IORESOURCE_TYPE_BITS;
+		u64 end = range.cpu_addr + range.size - 1;
+		dev_dbg(port->dev, "0x%08x 0x%016llx..0x%016llx -> 0x%016llx\n",
+			range.flags, range.cpu_addr, end, range.pci_addr);
+
+		switch (restype) {
+		case IORESOURCE_IO:
+			res = &port->io.res;
+			port->io.pci_addr = range.pci_addr;
+			of_pci_range_to_resource(&range, np, res);
+			xgene_pcie_setup_ob_reg(port, OMR1BARL, restype);
+			break;
+		case IORESOURCE_MEM:
+			res = &port->mem.res;
+			port->mem.pci_addr = range.pci_addr;
+			of_pci_range_to_resource(&range, np, res);
+			xgene_pcie_setup_ob_reg(port, OMR2BARL, restype);
+			break;
+		default:
+			dev_err(dev, "invalid io resource!");
+			return -EINVAL;
+		}
+	}
+	xgene_pcie_setup_cfg_reg(port->csr_base, cfg_addr);
+	return 0;
+}
+
+static void xgene_pcie_setup_pims(void *addr, u64 pim, u64 size)
+{
+	writel(lower_32_bits(pim), addr);
+	writel(upper_32_bits(pim) | EN_COHERENCY, addr + 0x04);
+	writel(lower_32_bits(size), addr + 0x10);
+	writel(upper_32_bits(size), addr + 0x14);
+}
+
+/*
+ * X-Gene PCIe support maximum 3 inbound memory regions
+ * This function helps to select a region based on size of region
+ */
+static int xgene_pcie_select_ib_reg(u8 *ib_reg_mask, u64 size)
+{
+	if ((size > 4) && (size < SZ_16M) && !(*ib_reg_mask & (1 << 1))) {
+		*ib_reg_mask |= (1 << 1);
+		return 1;
+	}
+
+	if ((size > SZ_1K) && (size < SZ_1T) && !(*ib_reg_mask & (1 << 0))) {
+		*ib_reg_mask |= (1 << 0);
+		return 0;
+	}
+
+	if ((size > SZ_1M) && (size < SZ_1T) && !(*ib_reg_mask & (1 << 2))) {
+		*ib_reg_mask |= (1 << 2);
+		return 2;
+	}
+	return -EINVAL;
+}
+
+static void xgene_pcie_setup_ib_reg(struct xgene_pcie_port *port,
+				    struct of_pci_range *range, u8 *ib_reg_mask)
+{
+	void __iomem *csr_base = port->csr_base;
+	void __iomem *cfg_base = port->cfg_base;
+	void *bar_addr;
+	void *pim_addr;
+	u64 restype = range->flags & IORESOURCE_TYPE_BITS;
+	u64 cpu_addr = range->cpu_addr;
+	u64 pci_addr = range->pci_addr;
+	u64 size = range->size;
+	u64 mask = ~(size - 1) | EN_REG;
+	u32 flags = PCI_BASE_ADDRESS_MEM_TYPE_64;
+	u32 bar_low;
+	int region;
+
+	region = xgene_pcie_select_ib_reg(ib_reg_mask, range->size);
+	if (region < 0) {
+		dev_warn(port->dev, "invalid pcie dma-range config\n");
+		return;
+	}
+
+	if (restype == PCI_BASE_ADDRESS_MEM_PREFETCH)
+		flags |= PCI_BASE_ADDRESS_MEM_PREFETCH;
+
+	bar_low = pcie_bar_low_val((u32)cpu_addr, flags);
+	switch (region) {
+	case 0:
+		xgene_pcie_set_ib_mask(csr_base, BRIDGE_CFG_4, flags, size);
+		bar_addr = cfg_base + PCI_BASE_ADDRESS_0;
+		writel(bar_low, bar_addr);
+		writel(upper_32_bits(cpu_addr), bar_addr + 0x4);
+		pim_addr = csr_base + PIM1_1L;
+		break;
+	case 1:
+		bar_addr = csr_base + IBAR2;
+		writel(bar_low, bar_addr);
+		writel(lower_32_bits(mask), csr_base + IR2MSK);
+		pim_addr = csr_base + PIM2_1L;
+		break;
+	case 2:
+		bar_addr = csr_base + IBAR3L;
+		writel(bar_low, bar_addr);
+		writel(upper_32_bits(cpu_addr), bar_addr + 0x4);
+		writel(lower_32_bits(mask), csr_base + IR3MSKL);
+		writel(upper_32_bits(mask), csr_base + IR3MSKL + 0x4);
+		pim_addr = csr_base + PIM3_1L;
+		break;
+	}
+
+	xgene_pcie_setup_pims(pim_addr, pci_addr, size);
+}
+
+static int pci_dma_range_parser_init(struct of_pci_range_parser *parser,
+				     struct device_node *node)
+{
+	const int na = 3, ns = 2;
+	int rlen;
+
+	parser->node = node;
+	parser->pna = of_n_addr_cells(node);
+	parser->np = parser->pna + na + ns;
+
+	parser->range = of_get_property(node, "dma-ranges", &rlen);
+	if (!parser->range)
+		return -ENOENT;
+
+	parser->end = parser->range + rlen / sizeof(__be32);
+	return 0;
+}
+
+static int xgene_pcie_parse_map_dma_ranges(struct xgene_pcie_port *port)
+{
+	struct device_node *np = port->node;
+	struct of_pci_range range;
+	struct of_pci_range_parser parser;
+	struct device *dev = port->dev;
+	u8 ib_reg_mask = 0;
+
+	if (pci_dma_range_parser_init(&parser, np)) {
+		dev_err(dev, "missing dma-ranges property\n");
+		return -EINVAL;
+	}
+
+	/* Get the dma-ranges from DT */
+	for_each_of_pci_range(&parser, &range) {
+		u64 end = range.cpu_addr + range.size - 1;
+		dev_dbg(port->dev, "0x%08x 0x%016llx..0x%016llx -> 0x%016llx\n",
+			range.flags, range.cpu_addr, end, range.pci_addr);
+		xgene_pcie_setup_ib_reg(port, &range, &ib_reg_mask);
+	}
+	return 0;
+}
+
+static int xgene_pcie_setup(int nr, struct pci_sys_data *sys)
+{
+	struct xgene_pcie_port *pp = sys->private_data;
+	struct resource *io = &pp->realio;
+
+	io->start = sys->domain * SZ_64K;
+	io->end = io->start + SZ_64K;
+	io->flags = pp->io.res.flags;
+	io->name = "PCI IO";
+	pci_ioremap_io(io->start, pp->io.res.start);
+
+	pci_add_resource_offset(&sys->resources, io, sys->io_offset);
+	sys->mem_offset = pp->mem.res.start - pp->mem.pci_addr;
+	pci_add_resource_offset(&sys->resources, &pp->mem.res,
+				sys->mem_offset);
+	return 1;
+}
+
+static int __init xgene_pcie_probe_bridge(struct platform_device *pdev)
+{
+	struct device_node *np = of_node_get(pdev->dev.of_node);
+	struct xgene_pcie_port *port;
+	struct hw_pci xgene_pcie_hw;
+	u32 lanes = 0, speed = 0;
+	u64 cfg_addr = 0;
+	static u32 domain;
+	int ret;
+
+	port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
+	if (!port)
+		return -ENOMEM;
+	port->node = np;
+	port->dev = &pdev->dev;
+
+	ret = xgene_pcie_map_reg(port, pdev, &cfg_addr);
+	if (ret)
+		return ret;
+
+	ret = xgene_pcie_init_port(port);
+	if (ret)
+		goto skip;
+	xgene_pcie_program_core(port->csr_base);
+	xgene_pcie_setup_root_complex(port);
+	ret = xgene_pcie_parse_map_ranges(port, cfg_addr);
+	if (ret)
+		goto skip;
+	ret = xgene_pcie_parse_map_dma_ranges(port);
+	if (ret)
+		goto skip;
+	xgene_pcie_poll_linkup(port, &lanes, &speed);
+skip:
+	if (!port->link_up)
+		dev_info(port->dev, "(rc) link down\n");
+	else
+		dev_info(port->dev, "(rc) x%d gen-%d link up\n",
+				lanes, speed + 1);
+	platform_set_drvdata(pdev, port);
+	memset(&xgene_pcie_hw, 0, sizeof(xgene_pcie_hw));
+	xgene_pcie_hw.domain = domain++;
+	xgene_pcie_hw.private_data = (void **)&port;
+	xgene_pcie_hw.nr_controllers = 1;
+	xgene_pcie_hw.setup = xgene_pcie_setup;
+	xgene_pcie_hw.map_irq = of_irq_parse_and_map_pci;
+	xgene_pcie_hw.ops = &xgene_pcie_ops;
+	pci_common_init(&xgene_pcie_hw);
+	return 0;
+}
+
+static const struct of_device_id xgene_pcie_match_table[] __initconst = {
+	{.compatible = "apm,xgene-pcie",},
+	{},
+};
+
+static struct platform_driver xgene_pcie_driver = {
+	.driver = {
+		   .name = "xgene-pcie",
+		   .owner = THIS_MODULE,
+		   .of_match_table = of_match_ptr(xgene_pcie_match_table),
+		  },
+	.probe = xgene_pcie_probe_bridge,
+};
+module_platform_driver(xgene_pcie_driver);
+
+MODULE_AUTHOR("Tanmay Inamdar <tinamdar@apm.com>");
+MODULE_DESCRIPTION("APM X-Gene PCIe driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

^ permalink raw reply related

* [RFC PATCH V3 0/4] APM X-Gene PCIe controller
From: Tanmay Inamdar @ 2014-01-24 21:32 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds support for AppliedMicro X-Gene PCIe host controller. The
driver is tested on X-Gene platform with different gen1/2/3 PCIe endpoint
cards.

X-Gene PCIe controller driver has depedency on the pcie arch support for
arm64. The arm64 pcie arch support is not yet part of mainline Linux kernel
and approach for arch support is under discussion with arm64 maintainers.
The reference patch can be found here --> https://lkml.org/lkml/2013/10/23/244

If someone wishes to test PCIe on X-Gene, arch support patch must be applied
before the patches in this patch set.

changes since V2:
1. redefined each PCI port in different PCI domain correctly.
2. removed setup_lane and setup_link functions from driver.
3. removed scan_bus wrapper and set_primary_bus hack.
4. added pci_ioremap_io for io resources.

changes since V1:
1. added PCI domain support
2. reading cpu and pci addresses from device tree to configure regions.
3. got rid of unnecessary wrappers for readl and writel.
4. got rid of endpoint configuration code.
5. added 'dma-ranges' property support to read inbound region configuration.
6. renamed host driver file to 'pci-xgene.c' from 'pcie-xgene.c'
7. dropped 'clock-names' property from bindings
8. added comments whereever requested.

Tanmay Inamdar (4):
  pci: APM X-Gene PCIe controller driver
  arm64: dts: APM X-Gene PCIe device tree nodes
  dt-bindings: pci: xgene pcie device tree bindings
  MAINTAINERS: entry for APM X-Gene PCIe host driver

 .../devicetree/bindings/pci/xgene-pci.txt          |   52 ++
 MAINTAINERS                                        |    7 +
 arch/arm64/boot/dts/apm-mustang.dts                |    8 +
 arch/arm64/boot/dts/apm-storm.dtsi                 |  155 ++++
 drivers/pci/host/Kconfig                           |   10 +
 drivers/pci/host/Makefile                          |    1 +
 drivers/pci/host/pci-xgene.c                       |  784 ++++++++++++++++++++
 7 files changed, 1017 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/xgene-pci.txt
 create mode 100644 drivers/pci/host/pci-xgene.c

-- 
1.7.9.5

^ permalink raw reply

* iMX6SL Suspend mode (S3)
From: John Tobias @ 2014-01-24 21:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140124212410.GH15937@n2100.arm.linux.org.uk>

Thanks Russell. Got it working.

Regards,

john

On Fri, Jan 24, 2014 at 1:24 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Jan 24, 2014 at 01:14:08PM -0800, John Tobias wrote:
>> Hello all,
>>
>> I am testing the suspend mode (S3) for my custom board with iMX6SL and
>> the iMX6SL EVK as well. I am using 3.13 kernel, eMMC and MMC for the
>> storage and systemd to control the OS. If I put my device in S3 mode,
>> I got the following messages:
>>
>> [   27.949393] PM: Syncing filesystems ... done.
>> [   27.972392] mmc0: card 0001 removed
>> [   28.033613] end_request: I/O error, dev mmcblk0, sector 0
>> [   28.063493] end_request: I/O error, dev mmcblk0, sector 0
>> [   28.107933] end_request: I/O error, dev mmcblk0, sector 0
>> [   28.131456] end_request: I/O error, dev mmcblk0, sector 0
>> [   28.153651] end_request: I/O error, dev mmcblk0, sector 0
>> [   28.215530] Freezing user space processes ... (elapsed 0.001 seconds) done.
>> [   28.224284] Freezing remaining freezable tasks ... (elapsed 0.001
>> seconds) done.
>> [   28.233658] Suspending console(s) (use no_console_suspend to debug)
>>
>> If I press any key, it will wake up. But, I got the following errors:
>>
>> [   17.815008] mmc0: new high speed DDR MMC card at address 0001
>> [   17.828024] mmcblk0: mmc0:0001 SEM04G 3.68 GiB
>> [   17.833522] mmcblk0boot0: mmc0:0001 SEM04G partition 1 2.00 MiB
>> [   17.839699] mmcblk0boot1: mmc0:0001 SEM04G partition 2 2.00 MiB
>> [   17.847150] mmcblk0rpmb: mmc0:0001 SEM04G partition 3 2.00 MiB
>> [   17.860116]  mmcblk0: p1 p2 p3 p4 < p5 p6 >
>> [   17.883550]  mmcblk0boot1: unknown partition table
>> [   17.899549]  mmcblk0boot0: unknown partition table
>
> This is because MMC insists that when you suspend, the card will be
> ejected, and when you resume, it will be re-inserted - it's effectively
> the same as if you pulled the card out and then plugged it back in
> without shutting the system down.
>
> This is the default for the MMC subsystem, unless MMC_UNSAFE_RESUME
> is set - and you can't have rootfs in MMC and suspend/resume unless
> that's set.
>
> --
> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
> Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* [RFC PATCH V2 1/4] pci: APM X-Gene PCIe controller driver
From: Tanmay Inamdar @ 2014-01-24 21:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACoXjc=ZT9fEm_KdY4WFSoS8n5FmO+hn9LjsR4m8YZG2iBbt5A@mail.gmail.com>

On Thu, Jan 16, 2014 at 5:10 PM, Tanmay Inamdar <tinamdar@apm.com> wrote:
> On Wed, Jan 15, 2014 at 4:39 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Wednesday 15 January 2014, Tanmay Inamdar wrote:
>>> This patch adds the AppliedMicro X-Gene SOC PCIe controller driver.
>>> X-Gene PCIe controller supports maxmum upto 8 lanes and GEN3 speed.
>>> X-Gene has maximum 5 PCIe ports supported.
>>>
>>> Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
>>
>> This already looks much better than the first version, but I have a more
>> comments. Most importantly, it would help to know how the root ports
>> are structured. Is this a standard root complex and multiple ports,
>> multiple root complexes with one port each, or a nonstandard organization
>> that is a mix of those two models?
>
> This is multiple root complexes with one port each.
>
>>
>>> +
>>> +/* When the address bit [17:16] is 2'b01, the Configuration access will be
>>> + * treated as Type 1 and it will be forwarded to external PCIe device.
>>> + */
>>> +static void __iomem *xgene_pcie_get_cfg_base(struct pci_bus *bus)
>>> +{
>>> +     struct xgene_pcie_port *port = xgene_pcie_bus_to_port(bus);
>>> +     u64 addr = (u64)port->cfg_base;
>>> +
>>> +     if (bus->number >= (port->first_busno + 1))
>>> +             addr |= AXI_EP_CFG_ACCESS;
>>> +
>>> +     return (void *)addr;
>>> +}
>>
>> Wrong type, it should be 'void __iomem *'. Also you can't assume that
>> bit operations work on virtual __iomem addresses, so it should be better
>> to just add a constant integer to the pointer, which is a valid
>> operation.
>
> ok.
>
>>
>> I also wonder why you need to do this at all. If there isn't a global
>> config space for all ports, but rather a separate type0/type1 config
>> cycle based on the bus number, I see that as an indication that the
>> ports are in fact separate domains and should each start with bus 0.
>
> It is not a standard ECAM layout. We also have a separate RTDID
> register as well to program bus, device, function. While accessing EP
> config space, we have to set the bit 17:16 as 2b'01. The same config
> space address is utilized for enabling a customized nonstandard PCIe
> DMA feature. The bits are defined to differentiate the access purpose.
> The feature is not supported in this driver yet.
>
> Secondly I don't think it will matter if each port starts with bus 0.
> As long as we set the correct BDF in RTDID and set correct bits in
> config address, the config reads and writes would work. Right?
>
>>
>>> +static void xgene_pcie_setup_lanes(struct xgene_pcie_port *port)
>>> +{
>>> +     void *csr_base = port->csr_base;
>>> +     u32 val;
>>> +
>>> +     val = readl(csr_base + BRIDGE_8G_CFG_8);
>>> +     val = eq_pre_cursor_lane0_set(val, 0x7);
>>> +     val = eq_pre_cursor_lane1_set(val, 0x7);
>>> +     writel(val, csr_base + BRIDGE_8G_CFG_8);
>>> +
>>> +     val = readl(csr_base + BRIDGE_8G_CFG_9);
>>> +     val = eq_pre_cursor_lane0_set(val, 0x7);
>>> +     val = eq_pre_cursor_lane1_set(val, 0x7);
>>> +     writel(val, csr_base + BRIDGE_8G_CFG_9);
>>> +
>>> +     val = readl(csr_base + BRIDGE_8G_CFG_10);
>>> +     val = eq_pre_cursor_lane0_set(val, 0x7);
>>> +     val = eq_pre_cursor_lane1_set(val, 0x7);
>>> +     writel(val, csr_base + BRIDGE_8G_CFG_10);
>>> +
>>> +     val = readl(csr_base + BRIDGE_8G_CFG_11);
>>> +     val = eq_pre_cursor_lane0_set(val, 0x7);
>>> +     val = eq_pre_cursor_lane1_set(val, 0x7);
>>> +     writel(val, csr_base + BRIDGE_8G_CFG_11);
>>> +
>>> +     val = readl(csr_base + BRIDGE_8G_CFG_4);
>>> +     val = (val & ~0x30) | (1 << 4);
>>> +     writel(val, csr_base + BRIDGE_8G_CFG_4);
>>> +}
>>
>> Please document what you are actually setting here. If the configuration
>> of the lanes is always the same, why do you have to set it here. If not,
>> why do you set constant values?
>
> Good point. Let me check if these values should be constant or tune-able.
>
>>
>>> +static void xgene_pcie_setup_link(struct xgene_pcie_port *port)
>>> +{
>>> +     void *csr_base = port->csr_base;
>>> +     u32 val;
>>> +
>>> +     val = readl(csr_base + BRIDGE_CFG_14);
>>> +     val |= DIRECT_TO_8GTS_MASK;
>>> +     val |= SUPPORT_5GTS_MASK;
>>> +     val |= SUPPORT_8GTS_MASK;
>>> +     val |= DIRECT_TO_5GTS_MASK;
>>> +     writel(val, csr_base + BRIDGE_CFG_14);
>>> +
>>> +     val = readl(csr_base + BRIDGE_CFG_14);
>>> +     val &= ~ADVT_INFINITE_CREDITS;
>>> +     writel(val, csr_base + BRIDGE_CFG_14);
>>> +
>>> +     val = readl(csr_base + BRIDGE_8G_CFG_0);
>>> +     val |= (val & ~0xf) | 7;
>>> +     val |= (val & ~0xf00) | ((7 << 8) & 0xf00);
>>> +     writel(val, csr_base + BRIDGE_8G_CFG_0);
>>> +
>>> +     val = readl(csr_base + BRIDGE_8G_CFG_0);
>>> +     val |= DWNSTRM_EQ_SKP_PHS_2_3;
>>> +     writel(val, csr_base + BRIDGE_8G_CFG_0);
>>> +}
>>
>> Same here.
>>
>>> +static void xgene_pcie_program_core(void *csr_base)
>>> +{
>>> +     u32 val;
>>> +
>>> +     val = readl(csr_base + BRIDGE_CFG_0);
>>> +     val |= AER_OPTIONAL_ERROR_EN;
>>> +     writel(val, csr_base + BRIDGE_CFG_0);
>>> +     writel(0x0, csr_base + INTXSTATUSMASK);
>>> +     val = readl(csr_base + BRIDGE_CTRL_1);
>>> +     val = (val & ~0xffff) | XGENE_PCIE_DEV_CTRL;
>>> +     writel(val, csr_base + BRIDGE_CTRL_1);
>>> +}
>>
>> 'program_core'?
>
> Some of the PCIe core related misc configurations.
>
>>
>>> +static void xgene_pcie_poll_linkup(struct xgene_pcie_port *port, u32 *lanes)
>>> +{
>>> +     void *csr_base = port->csr_base;
>>> +     u32 val32;
>>> +     u64 start_time, time;
>>> +
>>> +     /*
>>> +      * A component enters the LTSSM Detect state within
>>> +      * 20ms of the end of fundamental core reset.
>>> +      */
>>> +     msleep(XGENE_LTSSM_DETECT_WAIT);
>>> +     port->link_up = 0;
>>> +     start_time = jiffies;
>>> +     do {
>>> +             val32 = readl(csr_base + PCIECORE_CTLANDSTATUS);
>>> +             if (val32 & LINK_UP_MASK) {
>>> +                     port->link_up = 1;
>>> +                     port->link_speed = PIPE_PHY_RATE_RD(val32);
>>> +                     val32 = readl(csr_base + BRIDGE_STATUS_0);
>>> +                     *lanes = val32 >> 26;
>>> +             }
>>> +             time = jiffies_to_msecs(jiffies - start_time);
>>> +     } while ((!port->link_up) || (time <= XGENE_LTSSM_L0_WAIT));
>>> +}
>>
>> Maybe another msleep() in the loop? It seems weird to first do an
>> unconditional sleep but then busy-wait for the result.
>
> ok.

This loop can execute for maximum 4 msec. So putting msleep(1) won't
get us much.

>
>>
>>> +static void xgene_pcie_setup_primary_bus(struct xgene_pcie_port *port,
>>> +                                      u32 first_busno, u32 last_busno)
>>> +{
>>> +     u32 val;
>>> +     void *cfg_addr = port->cfg_base;
>>> +
>>> +     val = readl(cfg_addr + PCI_PRIMARY_BUS);
>>> +     val &= ~PCI_PRIMARY_BUS_MASK;
>>> +     val |= (last_busno << 16) | ((first_busno + 1) << 8) | (first_busno);
>>> +     writel(val, cfg_addr + PCI_PRIMARY_BUS);
>>> +}
>>
>> Please explain what you are doing here. As mentioned above, I would expect
>> that each domain has visibility of all 255 buses. You shouldn't need any hacks
>> where you try to artificially squeeze the ports into a single domain when
>> they are separate in hardware.
>
> ok. I will check and get back.

You are right. I have removed this hack. It will be fixed in next version.

>
>>
>>> +/*
>>> + * read configuration values from DTS
>>> + */
>>> +static int xgene_pcie_read_dts_config(struct xgene_pcie_port *port)
>>
>> The comment and function name don't seem to match what the function
>> does. The main purpose of this function seems to be to ioremap
>> the resources, which have nothing to with configuration.
>
> ok.
>
>>
>>> +{
>>> +     struct device_node *np = port->node;
>>> +     struct resource csr_res;
>>> +     struct resource cfg_res;
>>> +
>>> +     /* Get CSR space registers address */
>>> +     if (of_address_to_resource(np, 0, &csr_res))
>>> +             return -EINVAL;
>>> +
>>> +     port->csr_base = devm_ioremap_nocache(port->dev, csr_res.start,
>>> +                                           resource_size(&csr_res));
>>
>> You can also use platform_get_resource() to access the resource
>> that is already there, rather than creating another one.
>
> ok.
>
>>
>>> +static void xgene_pcie_setup_ob_reg(struct xgene_pcie_port *port,
>>> +                                 u32 addr, u32 restype)
>>> +{
>>> +     struct resource *res = NULL;
>>> +     void *base = port->csr_base + addr;
>>> +     resource_size_t size;
>>> +     u64 cpu_addr = 0;
>>> +     u64 pci_addr = 0;
>>> +     u64 mask = 0;
>>> +     u32 min_size = 0;
>>
>> A general note: don't initialize local variables to a bogus valus (e.g. 0)
>> in their declaration. It prevents the compiler from warning about
>> incorrect uses.
>
> ok.
>
>>
>>> +     u32 flag = EN_REG;
>>
>> This one on the other hand is ok, because you are actually going to
>> use that value.
>>
>>> +     switch (restype) {
>>> +     case IORESOURCE_MEM:
>>> +             res = &port->mem.res;
>>> +             pci_addr = port->mem.pci_addr;
>>> +             min_size = SZ_128M;
>>> +             break;
>>> +     case IORESOURCE_IO:
>>> +             res = &port->io.res;
>>> +             pci_addr = port->io.pci_addr;
>>> +             min_size = 128;
>>> +             flag |= OB_LO_IO;
>>> +             break;
>>> +     }
>>
>> I assume this works ok, but seems wrong in one detail: If the resource
>> is marked IORESOURCE_IO, res->start is supposed to be in I/O space, not
>> in memory space, which would make it the wrong number to program
>> into the hardware registers.
>
> Yes for using ioport resource. However we have decided to defer using
> it since 'pci_ioremap_io' is not yet supported from arm64 side.
>
> From HW point of view, for memory mapped IO space, it is nothing but a
> piece taken out of the ranges in address map for outbound accesses. So
> while configuring registers from SOC side, it should take the CPU
> address which is address from SOC address map. Right?
>
> Later on we can have a separate io resource like 'realio' similar to
> what pci-mvebu.c does.
>
>>
>>> +static int xgene_pcie_parse_map_ranges(struct xgene_pcie_port *port)
>>> +{
>>> +     struct device_node *np = port->node;
>>> +     struct of_pci_range range;
>>> +     struct of_pci_range_parser parser;
>>> +     struct device *dev = port->dev;
>>> +
>>> +     if (of_pci_range_parser_init(&parser, np)) {
>>> +             dev_err(dev, "missing ranges property\n");
>>> +             return -EINVAL;
>>> +     }
>>> +
>>> +     /* Get the I/O, memory, config ranges from DT */
>>
>> The comment needs updating now that you don't read config space here any more.
>
> ok.
>
>>
>>> +/* X-Gene PCIe support maximum 3 inbound memory regions
>>> + * This function helps to select a region based on size of region
>>> + */
>>> +static int xgene_pcie_select_ib_reg(u64 size)
>>> +{
>>> +     static u8 ib_reg_mask;
>>> +
>>> +     if ((size > 4) && (size < SZ_16M) && !(ib_reg_mask & (1 << 1))) {
>>> +             ib_reg_mask |= (1 << 1);
>>> +             return 1;
>>> +     }
>>> +
>>> +     if ((size > SZ_1K) && (size < SZ_1T) && !(ib_reg_mask & (1 << 0))) {
>>> +             ib_reg_mask |= (1 << 0);
>>> +             return 0;
>>> +     }
>>> +
>>> +     if ((size > SZ_1M) && (size < SZ_1T) && !(ib_reg_mask & (1 << 2))) {
>>> +             ib_reg_mask |= (1 << 2);
>>> +             return 2;
>>> +     }
>>> +     return -EINVAL;
>>> +}
>>
>> Shouldn't the ib_reg_mask variable be per host bridge? Static variables
>> are dangerous if you ever get multiple instances of the hardware in one
>> system.
>
> Yes. You are right. Thanks.
>
>>
>>> +static int xgene_pcie_parse_map_dma_ranges(struct xgene_pcie_port *port)
>>> +{
>>> +     struct device_node *np = port->node;
>>> +     struct of_pci_range range;
>>> +     struct of_pci_range_parser parser;
>>> +     struct device *dev = port->dev;
>>> +     int region;
>>> +
>>> +     if (pci_dma_range_parser_init(&parser, np)) {
>>> +             dev_err(dev, "missing dma-ranges property\n");
>>> +             return -EINVAL;
>>> +     }
>>> +
>>> +     /* Get the dma-ranges from DT */
>>> +     for_each_of_pci_range(&parser, &range) {
>>> +             u64 restype = range.flags & IORESOURCE_TYPE_BITS;
>>> +             u64 end = range.cpu_addr + range.size - 1;
>>> +             dev_dbg(port->dev, "0x%08x 0x%016llx..0x%016llx -> 0x%016llx\n",
>>> +                     range.flags, range.cpu_addr, end, range.pci_addr);
>>> +             region = xgene_pcie_select_ib_reg(range.size);
>>> +             if (region == -EINVAL) {
>>> +                     dev_warn(port->dev, "invalid pcie dma-range config\n");
>>> +                     continue;
>>> +             }
>>> +             xgene_pcie_setup_ib_reg(port, &range, restype, region);
>>> +     }
>>> +     return 0;
>>> +}
>>
>> I guess is could even be a local variable in this function, which you pass
>> by reference.
>>
>>> +
>>> +static int xgene_pcie_setup(int nr, struct pci_sys_data *sys)
>>> +{
>>> +     struct xgene_pcie_port *pp = xgene_pcie_sys_to_port(sys);
>>> +
>>> +     if (pp == NULL)
>>> +             return 0;
>>> +
>>> +     sys->mem_offset = pp->mem.res.start - pp->mem.pci_addr;
>>> +     pci_add_resource_offset(&sys->resources, &pp->mem.res,
>>> +                             sys->mem_offset);
>>> +     return 1;
>>> +}
>>
>> Please follow the regular error handling conventions, which are to
>> pass a negative errno value on error and zero on success.
>
> ok.
>
>>
>> Also, what would be a reason for the port to be zero here? If
>> it's something that can't happen in practice, don't try to handle
>> it gracefully. You can use BUG_ON() for fatal conditions that
>> are supposed to be impossible to reach.
>
> This function is a hook to upper layer. We register nr_controllers in
> hw_pci structure as MAX_PORTS we support. It can happen that number of
> ports actually enabled from device tree are less than the number in
> nr_controllers.
>
>>
>>> +static struct pci_bus __init *xgene_pcie_scan_bus(int nr,
>>> +                                               struct pci_sys_data *sys)
>>> +{
>>> +     struct xgene_pcie_port *pp = xgene_pcie_sys_to_port(sys);
>>> +
>>> +     pp->first_busno = sys->busnr;
>>> +     xgene_pcie_setup_primary_bus(pp, sys->busnr, 0xff);
>>> +     return pci_scan_root_bus(NULL, sys->busnr, &xgene_pcie_ops,
>>> +                              sys, &sys->resources);
>>> +}
>>
>> You have a number of helper functions that don't seem to gain
>> much at all. Just move the call to pci_scan_root_bus() into
>> xgene_pcie_setup_primary_bus() here, and use that as the .scan
>> callback.
>>
>
> I can do that if I can get rid of setup_primary_bus api. Let me check.
>
>>
>>> +     if (!port->link_up)
>>> +             dev_info(port->dev, "(rc) link down\n");
>>> +     else
>>> +             dev_info(port->dev, "(rc) x%d gen-%d link up\n",
>>> +                             lanes, port->link_speed + 1);
>>> +#ifdef CONFIG_PCI_DOMAINS
>>> +     xgene_pcie_hw.domain++;
>>> +#endif
>>> +     xgene_pcie_hw.private_data[index++] = port;
>>> +     platform_set_drvdata(pdev, port);
>>> +     return 0;
>>> +}
>>
>> Do you have multiple domains or not? I don't see how it can work if you
>> make the domain setup conditional on a kernel configuration option.
>> If you in fact have multiple domains, make sure in Kconfig that
>> CONFIG_PCI_DOMAINS is enabled. Otherwise don't mess with the domain
>> number...
>
> It is enabled in Kconfig.
>
>>
>>> +static const struct of_device_id xgene_pcie_match_table[] __initconst = {
>>> +     {.compatible = "apm,xgene-pcie",},
>>> +     {},
>>> +};
>>
>> Another general note: Your "compatible" strings are rather unspecific.
>> Do you have a version number for this IP block? I suppose that it's related
>> to one that has been used in other chips before, or will be used in future
>> chips, if it's not actually licensed from some other company.
>
> I will have to check this.
>

We have decided to stick with current compatible string for now.

>>
>>> +static int __init xgene_pcie_init(void)
>>> +{
>>> +     void *private;
>>> +     int ret;
>>> +
>>> +     pr_info("X-Gene: PCIe driver\n");
>>> +
>>> +     /* allocate private data to keep xgene_pcie_port information */
>>> +     private = kzalloc((XGENE_PCIE_MAX_PORTS * sizeof(void *)), GFP_KERNEL);
>>
>> This should not be done unconditionally: There is no point in printing
>> a message or allocating memory if you don't actually run on a system
>> with this device.
>
> I am doing this here because I have one instance of hw_pci structure
> with multiple pcie controllers. I can't do it from probe since it will
> be called once per instance in device tree.
>
>>
>>> +     if (private == NULL)
>>> +             return -ENOMEM;
>>
>> Style: if you are testing for an object, just write 'if (private)' or
>> 'if (!private)', but don't compare against NULL.
>
> ok.
>
>>
>>> +     xgene_pcie_hw.private_data = private;
>>> +     ret = platform_driver_probe(&xgene_pcie_driver,
>>> +                                 xgene_pcie_probe_bridge);
>>> +     if (ret)
>>> +             return ret;
>>> +     pci_common_init(&xgene_pcie_hw);
>>> +     return 0;
>>
>> This seems wrong: You should not use platform_driver_probe() because
>> that has issues with deferred probing.
>
> I think 'platform_driver_probe' prevents the deferred probing.
> 'pci_common_init' needs to be called only once with current driver
> structure. The probes for all pcie ports should be finished (ports
> initialized) before 'pci_common_init' gets called.
>
>>
>>         Arnd

^ permalink raw reply

* [PATCH v5 1/8] ARM: brcmstb: add infrastructure for ARM-based Broadcom STB SoCs
From: Marc C @ 2014-01-24 21:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140124101407.GL15586@e106331-lin.cambridge.arm.com>

Hi Mark,

>> +static void __init brcmstb_init_early(void)
>> +{
>> +       add_preferred_console("ttyS", 0, "115200");
>> +}
>
> Is this really required?

I think I can drop this. It was a holdover from our older kernels.

>> +       /*
>> +       * set the reset vector to point to the secondary_startup
>> +       * routine
>> +       */
>> +       cpu_set_boot_addr(cpu, virt_to_phys(brcmstb_secondary_startup));
>> +
>> +       flush_cache_all();
>
> Why? What does the new CPU need before its caches are coherent and up?

Absolutely nothing! I should be able to drop this as well.

Regarding the CPU power-down sequence, I'll review it and make sure it follows the
"Processor power domain" sequence in the A15 TRM. For any deviations, I'll double-check
with our H/W designers to ensure there aren't any magic requirements unaccounted for.

Thank you for taking a deep-dive into the code! I'll make the appropriate modifications
per your suggestions.

Regards,
Marc C


On 01/24/2014 02:14 AM, Mark Rutland wrote:
> On Wed, Jan 22, 2014 at 03:30:45AM +0000, Marc Carino wrote:
>> The BCM7xxx series of Broadcom SoCs are used primarily in set-top boxes.
>>
>> This patch adds machine support for the ARM-based Broadcom SoCs.
>>
>> Signed-off-by: Marc Carino <marc.ceeeee@gmail.com>
>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  arch/arm/configs/multi_v7_defconfig |    1 +
>>  arch/arm/mach-bcm/Kconfig           |   14 ++
>>  arch/arm/mach-bcm/Makefile          |    4 +
>>  arch/arm/mach-bcm/brcmstb.c         |  110 ++++++++++++
>>  arch/arm/mach-bcm/brcmstb.h         |   38 ++++
>>  arch/arm/mach-bcm/headsmp-brcmstb.S |   34 ++++
>>  arch/arm/mach-bcm/hotplug-brcmstb.c |  334 +++++++++++++++++++++++++++++++++++
>>  7 files changed, 535 insertions(+), 0 deletions(-)
>>  create mode 100644 arch/arm/mach-bcm/brcmstb.c
>>  create mode 100644 arch/arm/mach-bcm/brcmstb.h
>>  create mode 100644 arch/arm/mach-bcm/headsmp-brcmstb.S
>>  create mode 100644 arch/arm/mach-bcm/hotplug-brcmstb.c
>>
>> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
>> index c1df4e9..7028d11 100644
>> --- a/arch/arm/configs/multi_v7_defconfig
>> +++ b/arch/arm/configs/multi_v7_defconfig
>> @@ -7,6 +7,7 @@ CONFIG_MACH_ARMADA_370=y
>>  CONFIG_MACH_ARMADA_XP=y
>>  CONFIG_ARCH_BCM=y
>>  CONFIG_ARCH_BCM_MOBILE=y
>> +CONFIG_ARCH_BRCMSTB=y
>>  CONFIG_GPIO_PCA953X=y
>>  CONFIG_ARCH_HIGHBANK=y
>>  CONFIG_ARCH_KEYSTONE=y
>> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
>> index 9fe6d88..2c1ae83 100644
>> --- a/arch/arm/mach-bcm/Kconfig
>> +++ b/arch/arm/mach-bcm/Kconfig
>> @@ -31,6 +31,20 @@ config ARCH_BCM_MOBILE
>>           BCM11130, BCM11140, BCM11351, BCM28145 and
>>           BCM28155 variants.
>>
>> +config ARCH_BRCMSTB
>> +       bool "Broadcom BCM7XXX based boards" if ARCH_MULTI_V7
>> +       depends on MMU
>> +       select ARM_GIC
>> +       select MIGHT_HAVE_PCI
>> +       select HAVE_SMP
>> +       select HAVE_ARM_ARCH_TIMER
>> +       help
>> +         Say Y if you intend to run the kernel on a Broadcom ARM-based STB
>> +         chipset.
>> +
>> +         This enables support for Broadcom ARM-based set-top box chipsets,
>> +         including the 7445 family of chips.
>> +
>>  endmenu
>>
>>  endif
>> diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
>> index c2ccd5a..b744a12 100644
>> --- a/arch/arm/mach-bcm/Makefile
>> +++ b/arch/arm/mach-bcm/Makefile
>> @@ -13,3 +13,7 @@
>>  obj-$(CONFIG_ARCH_BCM_MOBILE)  := board_bcm281xx.o bcm_kona_smc.o bcm_kona_smc_asm.o kona.o
>>  plus_sec := $(call as-instr,.arch_extension sec,+sec)
>>  AFLAGS_bcm_kona_smc_asm.o      :=-Wa,-march=armv7-a$(plus_sec)
>> +
>> +obj-$(CONFIG_ARCH_BRCMSTB)     := brcmstb.o
>> +obj-$(CONFIG_SMP)              += headsmp-brcmstb.o
>> +obj-$(CONFIG_HOTPLUG_CPU)      += hotplug-brcmstb.o
>> diff --git a/arch/arm/mach-bcm/brcmstb.c b/arch/arm/mach-bcm/brcmstb.c
>> new file mode 100644
>> index 0000000..7a6093d
>> --- /dev/null
>> +++ b/arch/arm/mach-bcm/brcmstb.c
>> @@ -0,0 +1,110 @@
>> +/*
>> + * Copyright (C) 2013 Broadcom Corporation
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/console.h>
>> +#include <linux/clocksource.h>
>> +#include <linux/delay.h>
>> +#include <linux/device.h>
>> +#include <linux/errno.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/jiffies.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/printk.h>
>> +#include <linux/smp.h>
>> +
>> +#include <asm/cacheflush.h>
>> +#include <asm/mach-types.h>
>> +#include <asm/mach/arch.h>
>> +#include <asm/mach/map.h>
>> +#include <asm/mach/time.h>
>> +
>> +#include "brcmstb.h"
>> +
>> +/***********************************************************************
>> + * STB CPU (main application processor)
>> + ***********************************************************************/
>> +
>> +static const char *brcmstb_match[] __initconst = {
>> +       "brcm,bcm7445",
>> +       "brcm,brcmstb",
>> +       NULL
>> +};
>> +
>> +static void __init brcmstb_init_early(void)
>> +{
>> +       add_preferred_console("ttyS", 0, "115200");
>> +}
> 
> Is this really required?
> 
>> +
>> +/***********************************************************************
>> + * SMP boot
>> + ***********************************************************************/
>> +
>> +#ifdef CONFIG_SMP
>> +static DEFINE_SPINLOCK(boot_lock);
>> +
>> +static void __cpuinit brcmstb_secondary_init(unsigned int cpu)
>> +{
>> +       /*
>> +        * Synchronise with the boot thread.
>> +        */
>> +       spin_lock(&boot_lock);
>> +       spin_unlock(&boot_lock);
>> +}
>> +
>> +static int __cpuinit brcmstb_boot_secondary(unsigned int cpu,
>> +                                           struct task_struct *idle)
>> +{
>> +       /*
>> +        * set synchronisation state between this boot processor
>> +        * and the secondary one
>> +        */
>> +       spin_lock(&boot_lock);
>> +
>> +       /* Bring up power to the core if necessary */
>> +       if (brcmstb_cpu_get_power_state(cpu) == 0)
>> +               brcmstb_cpu_power_on(cpu);
>> +
>> +       brcmstb_cpu_boot(cpu);
>> +
>> +       /*
>> +        * now the secondary core is starting up let it run its
>> +        * calibrations, then wait for it to finish
>> +        */
>> +       spin_unlock(&boot_lock);
>> +
>> +       return 0;
>> +}
>> +
>> +struct smp_operations brcmstb_smp_ops __initdata = {
>> +       .smp_prepare_cpus       = brcmstb_cpu_ctrl_setup,
>> +       .smp_secondary_init     = brcmstb_secondary_init,
>> +       .smp_boot_secondary     = brcmstb_boot_secondary,
>> +#ifdef CONFIG_HOTPLUG_CPU
>> +       .cpu_kill               = brcmstb_cpu_kill,
>> +       .cpu_die                = brcmstb_cpu_die,
>> +#endif
>> +};
>> +#endif
>> +
>> +DT_MACHINE_START(BRCMSTB, "Broadcom STB (Flattened Device Tree)")
>> +       .dt_compat      = brcmstb_match,
>> +#ifdef CONFIG_SMP
>> +       .smp            = smp_ops(brcmstb_smp_ops),
>> +#endif
>> +       .init_early     = brcmstb_init_early,
>> +MACHINE_END
>> diff --git a/arch/arm/mach-bcm/brcmstb.h b/arch/arm/mach-bcm/brcmstb.h
>> new file mode 100644
>> index 0000000..e49bde6
>> --- /dev/null
>> +++ b/arch/arm/mach-bcm/brcmstb.h
>> @@ -0,0 +1,38 @@
>> +/*
>> + * Copyright (C) 2013 Broadcom Corporation
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#ifndef __BRCMSTB_H__
>> +#define __BRCMSTB_H__
>> +
>> +#if !defined(__ASSEMBLY__)
>> +#include <linux/smp.h>
>> +#endif
>> +
>> +#if !defined(__ASSEMBLY__)
>> +extern void brcmstb_secondary_startup(void);
>> +extern void brcmstb_cpu_boot(unsigned int cpu);
>> +extern void brcmstb_cpu_power_on(unsigned int cpu);
>> +extern int brcmstb_cpu_get_power_state(unsigned int cpu);
>> +extern struct smp_operations brcmstb_smp_ops;
>> +#if defined(CONFIG_HOTPLUG_CPU)
>> +extern void brcmstb_cpu_die(unsigned int cpu);
>> +extern int brcmstb_cpu_kill(unsigned int cpu);
>> +void __init brcmstb_cpu_ctrl_setup(unsigned int max_cpus);
>> +#else
>> +static inline void brcmstb_cpu_die(unsigned int cpu) {}
>> +static inline int brcmstb_cpu_kill(unsigned int cpu) {}
>> +static inline void __init brcmstb_cpu_ctrl_setup(unsigned int max_cpus) {}
>> +#endif
>> +#endif
>> +
>> +#endif /* __BRCMSTB_H__ */
>> diff --git a/arch/arm/mach-bcm/headsmp-brcmstb.S b/arch/arm/mach-bcm/headsmp-brcmstb.S
>> new file mode 100644
>> index 0000000..57ec438
>> --- /dev/null
>> +++ b/arch/arm/mach-bcm/headsmp-brcmstb.S
>> @@ -0,0 +1,34 @@
>> +/*
>> + * SMP boot code for secondary CPUs
>> + * Based on arch/arm/mach-tegra/headsmp.S
>> + *
>> + * Copyright (C) 2010 NVIDIA, Inc.
>> + * Copyright (C) 2013 Broadcom Corporation
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <asm/assembler.h>
>> +#include <linux/linkage.h>
>> +#include <linux/init.h>
>> +
>> +        .section ".text.head", "ax"
>> +       __CPUINIT
> 
> __CPUINIT is either going or gone by now. This should disappear.
> 
>> +
>> +ENTRY(brcmstb_secondary_startup)
>> +        /*
>> +         * Ensure CPU is in a sane state by disabling all IRQs and switching
>> +         * into SVC mode.
>> +         */
>> +        setmode        PSR_I_BIT | PSR_F_BIT | SVC_MODE, r0
>> +
>> +        bl      v7_invalidate_l1
>> +        b       secondary_startup
>> +ENDPROC(brcmstb_secondary_startup)
>> diff --git a/arch/arm/mach-bcm/hotplug-brcmstb.c b/arch/arm/mach-bcm/hotplug-brcmstb.c
>> new file mode 100644
>> index 0000000..ff4a732
>> --- /dev/null
>> +++ b/arch/arm/mach-bcm/hotplug-brcmstb.c
>> @@ -0,0 +1,334 @@
>> +/*
>> + * Broadcom STB CPU hotplug support for ARM
>> + *
>> + * Copyright (C) 2013 Broadcom Corporation
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/device.h>
>> +#include <linux/errno.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/jiffies.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/printk.h>
>> +#include <linux/regmap.h>
>> +#include <linux/smp.h>
>> +#include <linux/mfd/syscon.h>
>> +
>> +#include <asm/cacheflush.h>
>> +#include <asm/mach-types.h>
>> +
>> +#include "brcmstb.h"
>> +
>> +enum {
>> +       ZONE_MAN_CLKEN_MASK             = BIT(0),
>> +       ZONE_MAN_RESET_CNTL_MASK        = BIT(1),
>> +       ZONE_MAN_MEM_PWR_MASK           = BIT(4),
>> +       ZONE_RESERVED_1_MASK            = BIT(5),
>> +       ZONE_MAN_ISO_CNTL_MASK          = BIT(6),
>> +       ZONE_MANUAL_CONTROL_MASK        = BIT(7),
>> +       ZONE_PWR_DN_REQ_MASK            = BIT(9),
>> +       ZONE_PWR_UP_REQ_MASK            = BIT(10),
>> +       ZONE_BLK_RST_ASSERT_MASK        = BIT(10),
>> +       ZONE_PWR_OFF_STATE_MASK         = BIT(26),
>> +       ZONE_PWR_ON_STATE_MASK          = BIT(26),
>> +       ZONE_DPG_PWR_STATE_MASK         = BIT(28),
>> +       ZONE_MEM_PWR_STATE_MASK         = BIT(29),
>> +       ZONE_RESET_STATE_MASK           = BIT(31),
>> +};
>> +
>> +static void __iomem *cpubiuctrl_block;
>> +static void __iomem *hif_cont_block;
>> +static u32 cpu0_pwr_zone_ctrl_reg;
>> +static u32 cpu_rst_cfg_reg;
>> +static u32 hif_cont_reg;
>> +DEFINE_PER_CPU(int, per_cpu_sw_state);
>> +
>> +static void __iomem *pwr_ctrl_get_base(unsigned int cpu)
>> +{
>> +       void __iomem *base = cpubiuctrl_block + cpu0_pwr_zone_ctrl_reg;
>> +       base += (cpu * 4);
> 
> CPU isn't guaranteed to be the physical CPU ID (MPIDR.Aff*). While it
> almost certainly will be, we can't guarantee it in the face of a kexec,
> for example.
> 
> You can use cpu_logical_map(cpu) to get the physical ID.
> 
>> +       return base;
>> +}
>> +
>> +static u32 pwr_ctrl_rd(unsigned int cpu)
>> +{
>> +       void __iomem *base = pwr_ctrl_get_base(cpu);
>> +       return readl_relaxed(base);
>> +}
>> +
>> +static void pwr_ctrl_wr(unsigned int cpu, u32 val)
>> +{
>> +       void __iomem *base = pwr_ctrl_get_base(cpu);
>> +       writel(val, base);
>> +}
>> +
>> +static void cpu_rst_cfg_set(int cpu, int set)
>> +{
>> +       u32 val;
>> +       val = readl_relaxed(cpubiuctrl_block + cpu_rst_cfg_reg);
>> +       if (set)
>> +               val |= BIT(cpu);
>> +       else
>> +               val &= ~BIT(cpu);
> 
> Likewise here.
> 
>> +       writel_relaxed(val, cpubiuctrl_block + cpu_rst_cfg_reg);
>> +}
>> +
>> +static void cpu_set_boot_addr(int cpu, unsigned long boot_addr)
>> +{
>> +       const int reg_ofs = cpu * 8;
> 
> And here.
> 
>> +       writel_relaxed(0, hif_cont_block + hif_cont_reg + reg_ofs);
>> +       writel_relaxed(boot_addr, hif_cont_block + hif_cont_reg + 4 + reg_ofs);
>> +}
>> +
>> +void brcmstb_cpu_boot(unsigned int cpu)
>> +{
>> +       pr_info("SMP: Booting CPU%d...\n", cpu);
>> +
>> +       /*
>> +       * set the reset vector to point to the secondary_startup
>> +       * routine
>> +       */
>> +       cpu_set_boot_addr(cpu, virt_to_phys(brcmstb_secondary_startup));
>> +
>> +       flush_cache_all();
> 
> Why? What does the new CPU need before its caches are coherent and up?
> 
>> +
>> +       /* unhalt the cpu */
>> +       cpu_rst_cfg_set(cpu, 0);
>> +}
>> +
>> +void brcmstb_cpu_power_on(unsigned int cpu)
>> +{
>> +       /*
>> +        * The secondary cores power was cut, so we must go through
>> +        * power-on initialization.
>> +        */
>> +       u32 tmp;
>> +
>> +       pr_info("SMP: Powering up CPU%d...\n", cpu);
>> +
>> +       /* Request zone power up */
>> +       pwr_ctrl_wr(cpu, ZONE_PWR_UP_REQ_MASK);
>> +
>> +       /* Wait for the power up FSM to complete */
>> +       do {
>> +               tmp = pwr_ctrl_rd(cpu);
>> +       } while (!(tmp & ZONE_PWR_ON_STATE_MASK));
>> +
>> +       per_cpu(per_cpu_sw_state, cpu) = 1;
>> +}
>> +
>> +int brcmstb_cpu_get_power_state(unsigned int cpu)
>> +{
>> +       int tmp = pwr_ctrl_rd(cpu);
>> +       return (tmp & ZONE_RESET_STATE_MASK) ? 0 : 1;
>> +}
>> +
>> +void __ref brcmstb_cpu_die(unsigned int cpu)
>> +{
>> +       /* Derived from misc_bpcm_arm.c */
>> +
>> +       /* Clear SCTLR.C bit */
>> +       __asm__(
>> +               "mrc    p15, 0, r0, c1, c0, 0\n"
>> +               "bic    r0, r0, #(1 << 2)\n"
>> +               "mcr    p15, 0, r0, c1, c0, 0\n"
>> +               : /* no output */
>> +               : /* no input */
>> +               : "r0"  /* clobber r0 */
>> +       );
> 
> This is odd. Why not allow GCC to allocate the register?
> 
>> +
>> +       /*
>> +        * Instruction barrier to ensure cache is really disabled before
>> +        * cleaning/invalidating the caches
>> +        */
>> +       isb();
> 
> I think you could use:
> 
> set_cr(get_cr() & ~CR_C))
> 
> Which would do all of the above (including the isb), and will get GCC to
> allocate the register.
> 
>> +
>> +       flush_cache_all();
>> +
>> +       /* Invalidate all instruction caches to PoU (ICIALLU) */
>> +       /* Data sync. barrier to ensure caches have emptied out */
>> +       __asm__("mcr    p15, 0, r0, c7, c5, 0\n" : : : "r0");
>> +       dsb();
> 
> Why do you need to invalidate the I-cache?
> 
>> +
>> +       /*
>> +        * Clear ACTLR.SMP bit to prevent broadcast TLB messages from reaching
>> +        * this core
>> +        */
>> +       __asm__(
>> +               "mrc    p15, 0, r0, c1, c0, 1\n"
>> +               "bic    r0, r0, #(1 << 6)\n"
>> +               "mcr    p15, 0, r0, c1, c0, 1\n"
>> +               : /* no output */
>> +               : /* no input */
>> +               : "r0"  /* clobber r0 */
>> +       );
> 
> Surely you can use an output operand to get GCC to allocate the register
> for you?
> 
>> +
>> +       /* Disable all IRQs for this CPU */
>> +       arch_local_irq_disable();
>> +
>> +       per_cpu(per_cpu_sw_state, cpu) = 0;
> 
> Your caches are off at this point, so this could be going straight to
> memory. Yet readers of this value aren't cleaning their caches before
> reading this, so they could hit a stale cached copy.
> 
>> +
>> +       /*
>> +        * Final full barrier to ensure everything before this instruction has
>> +        * quiesced.
>> +        */
>> +       isb();
>> +       dsb();
>> +
>> +       /* Sit and wait to die */
>> +       wfi();
>> +
>> +       /* We should never get here... */
>> +       nop();
> 
> Why the nop first?
> 
>> +       panic("Spurious interrupt on CPU %d received!\n", cpu);
>> +}
>> +
>> +int brcmstb_cpu_kill(unsigned int cpu)
>> +{
>> +       u32 tmp;
>> +
>> +       pr_info("SMP: Powering down CPU%d...\n", cpu);
>> +
>> +       while (per_cpu(per_cpu_sw_state, cpu))
>> +               ;
> 
> As this was written to with caches disabled, the cached copy of the
> value (which this is reading) could be stale. Surely you need to
> clean+invalidate the line for this value each time you read it to give
> it a chance to update?
> 
>> +
>> +       /* Program zone reset */
>> +       pwr_ctrl_wr(cpu, ZONE_RESET_STATE_MASK | ZONE_BLK_RST_ASSERT_MASK |
>> +                             ZONE_PWR_DN_REQ_MASK);
>> +
>> +       /* Verify zone reset */
>> +       tmp = pwr_ctrl_rd(cpu);
>> +       if (!(tmp & ZONE_RESET_STATE_MASK))
>> +               pr_err("%s: Zone reset bit for CPU %d not asserted!\n",
>> +                       __func__, cpu);
>> +
>> +       /* Wait for power down */
>> +       do {
>> +               tmp = pwr_ctrl_rd(cpu);
>> +       } while (!(tmp & ZONE_PWR_OFF_STATE_MASK));
>> +
>> +       /* Settle-time from Broadcom-internal DVT reference code */
>> +       udelay(7);
>> +
>> +       /* Assert reset on the CPU */
>> +       cpu_rst_cfg_set(cpu, 1);
>> +
>> +       return 1;
>> +}
>> +
>> +static int __init setup_hifcpubiuctrl_regs(struct device_node *np)
>> +{
>> +       int rc = 0;
>> +       char *name;
>> +       int index;
>> +       struct device_node *syscon_np = NULL;
>> +
>> +       name = "syscon-cpu";
>> +
>> +       syscon_np = of_parse_phandle(np, name, 0);
>> +       if (!syscon_np) {
>> +               pr_err("can't find phandle %s\n", name);
>> +               rc = -EINVAL;
>> +               goto cleanup;
>> +       }
>> +
>> +       cpubiuctrl_block = of_iomap(syscon_np, 0);
>> +       if (!cpubiuctrl_block) {
>> +               pr_err("iomap failed for cpubiuctrl_block\n");
>> +               rc = -EINVAL;
>> +               goto cleanup;
>> +       }
>> +
>> +       index = 1;
>> +       rc = of_property_read_u32_index(np, name, index,
>> +                                       &cpu0_pwr_zone_ctrl_reg);
> 
> The index variable seems rather pointless. Why not just use the value
> in-place?
> 
>> +       if (rc) {
>> +               pr_err("failed to read %d from %s property (%d)\n", index, name,
>> +                       rc);
> 
> It might be better to state _what_ you're looking for (what does the
> value represent?).
> 
>> +               rc = -EINVAL;
>> +               goto cleanup;
>> +       }
>> +
>> +       index = 2;
>> +       rc = of_property_read_u32_index(np, name, index, &cpu_rst_cfg_reg);
> 
> Likewise for all of the above.
> 
> Thanks,
> Mark.
> 

^ permalink raw reply

* iMX6SL Suspend mode (S3)
From: Russell King - ARM Linux @ 2014-01-24 21:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACUGKYMxozVYUZ1neX9iRqRoaTCTOOY8VV92UAtcwUa_1f0XSw@mail.gmail.com>

On Fri, Jan 24, 2014 at 01:14:08PM -0800, John Tobias wrote:
> Hello all,
> 
> I am testing the suspend mode (S3) for my custom board with iMX6SL and
> the iMX6SL EVK as well. I am using 3.13 kernel, eMMC and MMC for the
> storage and systemd to control the OS. If I put my device in S3 mode,
> I got the following messages:
> 
> [   27.949393] PM: Syncing filesystems ... done.
> [   27.972392] mmc0: card 0001 removed
> [   28.033613] end_request: I/O error, dev mmcblk0, sector 0
> [   28.063493] end_request: I/O error, dev mmcblk0, sector 0
> [   28.107933] end_request: I/O error, dev mmcblk0, sector 0
> [   28.131456] end_request: I/O error, dev mmcblk0, sector 0
> [   28.153651] end_request: I/O error, dev mmcblk0, sector 0
> [   28.215530] Freezing user space processes ... (elapsed 0.001 seconds) done.
> [   28.224284] Freezing remaining freezable tasks ... (elapsed 0.001
> seconds) done.
> [   28.233658] Suspending console(s) (use no_console_suspend to debug)
> 
> If I press any key, it will wake up. But, I got the following errors:
> 
> [   17.815008] mmc0: new high speed DDR MMC card at address 0001
> [   17.828024] mmcblk0: mmc0:0001 SEM04G 3.68 GiB
> [   17.833522] mmcblk0boot0: mmc0:0001 SEM04G partition 1 2.00 MiB
> [   17.839699] mmcblk0boot1: mmc0:0001 SEM04G partition 2 2.00 MiB
> [   17.847150] mmcblk0rpmb: mmc0:0001 SEM04G partition 3 2.00 MiB
> [   17.860116]  mmcblk0: p1 p2 p3 p4 < p5 p6 >
> [   17.883550]  mmcblk0boot1: unknown partition table
> [   17.899549]  mmcblk0boot0: unknown partition table

This is because MMC insists that when you suspend, the card will be
ejected, and when you resume, it will be re-inserted - it's effectively
the same as if you pulled the card out and then plugged it back in
without shutting the system down.

This is the default for the MMC subsystem, unless MMC_UNSAFE_RESUME
is set - and you can't have rootfs in MMC and suspend/resume unless
that's set.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* iMX6SL Suspend mode (S3)
From: John Tobias @ 2014-01-24 21:14 UTC (permalink / raw)
  To: linux-arm-kernel

Hello all,

I am testing the suspend mode (S3) for my custom board with iMX6SL and
the iMX6SL EVK as well. I am using 3.13 kernel, eMMC and MMC for the
storage and systemd to control the OS. If I put my device in S3 mode,
I got the following messages:

[   27.949393] PM: Syncing filesystems ... done.
[   27.972392] mmc0: card 0001 removed
[   28.033613] end_request: I/O error, dev mmcblk0, sector 0
[   28.063493] end_request: I/O error, dev mmcblk0, sector 0
[   28.107933] end_request: I/O error, dev mmcblk0, sector 0
[   28.131456] end_request: I/O error, dev mmcblk0, sector 0
[   28.153651] end_request: I/O error, dev mmcblk0, sector 0
[   28.215530] Freezing user space processes ... (elapsed 0.001 seconds) done.
[   28.224284] Freezing remaining freezable tasks ... (elapsed 0.001
seconds) done.
[   28.233658] Suspending console(s) (use no_console_suspend to debug)

If I press any key, it will wake up. But, I got the following errors:

[   17.815008] mmc0: new high speed DDR MMC card at address 0001
[   17.828024] mmcblk0: mmc0:0001 SEM04G 3.68 GiB
[   17.833522] mmcblk0boot0: mmc0:0001 SEM04G partition 1 2.00 MiB
[   17.839699] mmcblk0boot1: mmc0:0001 SEM04G partition 2 2.00 MiB
[   17.847150] mmcblk0rpmb: mmc0:0001 SEM04G partition 3 2.00 MiB
[   17.860116]  mmcblk0: p1 p2 p3 p4 < p5 p6 >
[   17.883550]  mmcblk0boot1: unknown partition table
[   17.899549]  mmcblk0boot0: unknown partition table
[   17.959186] EXT4-fs error (device mmcblk0p2):
__ext4_get_inode_loc:3909: inode #33300: block 131267: comm systemd:
unable to read itable block
[   18.126790] EXT4-fs error (device mmcblk0p2):
__ext4_get_inode_loc:3909: inode #33300: block 131267: comm systemd:
unable to read itable block
[   18.173370] EXT4-fs error (device mmcblk0p2):
__ext4_get_inode_loc:3909: inode #33300: block 131267: comm systemd:
unable to read itable block
[   18.207744] EXT4-fs error (device mmcblk0p2):
__ext4_get_inode_loc:3909: inode #33300: block 131267: comm systemd:
unable to read itable block
[   18.316736] EXT4-fs error (device mmcblk0p2):
__ext4_get_inode_loc:3909: inode #33300: block 131267: comm systemd:
unable to read itable block
[   18.448542] EXT4-fs error (device mmcblk0p2):
__ext4_get_inode_loc:3909: inode #33300: block 131267: comm systemd:
unable to read itable block
[   18.496749] EXT4-fs error (device mmcblk0p2):
__ext4_get_inode_loc:3909: inode #33300: block 131267: comm systemd:
unable to read itable block
[   18.530178] EXT4-fs error (device mmcblk0p2):
__ext4_get_inode_loc:3909: inode #33300: block 131267: comm systemd:
unable to read itable block
[   18.575861] EXT4-fs error (device mmcblk0p2):
__ext4_get_inode_loc:3909: inode #33300: block 131267: comm systemd:
unable to read itable block
[   18.660139] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:908: error reading directory block (ino 33142,
block 0)
[   18.717386] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:681: error reading directory block (ino 2001,
block 0)
[   18.830148] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:908: error reading directory block (ino 33135,
block 0)
[   19.045944] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:681: error reading directory block (ino 32928,
block 0)
[   19.058510] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:681: error reading directory block (ino 32928,
block 0)
[   19.071681] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:681: error reading directory block (ino 90, block
0)
[   19.083810] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:681: error reading directory block (ino 90, block
0)
[   19.097369] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:681: error reading directory block (ino 2001,
block 0)
[   19.110210] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:681: error reading directory block (ino 2001,
block 0)
[   19.153687] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:681: error reading directory block (ino 2001,
block 0)
[   29.800522] EXT4-fs warning: 1 callbacks suppressed
[   29.805425] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:681: error reading directory block (ino 2001,
block 0)
[   29.842343] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:681: error reading directory block (ino 90, block
0)
[   29.875698] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:681: error reading directory block (ino 32928,
block 0)
[   29.906757] EXT4-fs warning (device mmcblk0p2):
__ext4_read_dirblock:681: error reading directory block (ino 90, block
0)

Regards,

john

^ permalink raw reply

* [PATCH 09/20] ARM64 / ACPI: Implement core functions for parsing MADT table
From: Arnd Bergmann @ 2014-01-24 20:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52E287EE.70407@linaro.org>

On Friday 24 January 2014, Hanjun Guo wrote:
> >> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> >> index e108d9c..c335c6d 100644
> >> --- a/arch/arm64/include/asm/acpi.h
> >> +++ b/arch/arm64/include/asm/acpi.h
> >> @@ -83,6 +83,9 @@ void arch_fix_phys_package_id(int num, u32 slot);
> >>   extern int (*acpi_suspend_lowlevel)(void);
> >>   #define acpi_wakeup_address (0)
> >>   
> >> +#define MAX_GIC_CPU_INTERFACE 256
> > I'll bite. Where on Earth is this value coming from?
> 
> I just thought 256 is big enough for now :(
> Yes, should be a larger number for GICv3.

Could this just be set to NR_CPUS? That way it will be large enough for
any system you can actually run on.

	Arnd

^ permalink raw reply

* [PATCH V2 4/8] SPEAr13xx: Fixup: Move SPEAr1340 SATA platform code to system cfg driver
From: Arnd Bergmann @ 2014-01-24 20:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140124042959.GC2369@pratyush-vbox>

On Friday 24 January 2014, Pratyush Anand wrote:
> On Thu, Jan 23, 2014 at 08:22:54PM +0800, Arnd Bergmann wrote:
> > On Thursday 23 January 2014, Mohit Kumar wrote:
> > 
> > I assume you'd want a phandle pointing to the syscon device in here
> > as well?
> 
> Since there is only one syscon device in the whole DT, so do I really
> need to add phandle? Currently I am using
> syscon_regmap_lookup_by_compatible to find  syscon device.

I'd much rather use syscon_regmap_lookup_by_phandle than
syscon_regmap_lookup_by_compatible, all the time, since this makes
the relationship between the devices explicit.
 
The phandle method also allows you to pass regmap indexes in the
same property, which can be handy if two variants of the chip have
the same registers at a different offset.

> > > +/* SPEAr1340 Registers */
> > > +/* Power Management Registers */
> > > +#define SPEAR1340_PCM_CFG			0x100
> > > +	#define SPEAR1340_PCM_CFG_SATA_POWER_EN	0x800
> > > +#define SPEAR1340_PCM_WKUP_CFG			0x104
> > > +#define SPEAR1340_SWITCH_CTR			0x108
> > > +
> > > +#define SPEAR1340_PERIP1_SW_RST			0x318
> > > +	#define SPEAR1340_PERIP1_SW_RST_SATA	0x1000
> > > +#define SPEAR1340_PERIP2_SW_RST			0x31C
> > > +#define SPEAR1340_PERIP3_SW_RST			0x320
> > > +
> > > +/* PCIE - SATA configuration registers */
> > > +#define SPEAR1340_PCIE_SATA_CFG			0x424
> > > +	/* PCIE CFG MASks */
> > > +	#define SPEAR1340_PCIE_CFG_DEVICE_PRESENT	(1 << 11)
> > > +	#define SPEAR1340_PCIE_CFG_POWERUP_RESET	(1 << 10)
> > > +	#define SPEAR1340_PCIE_CFG_CORE_CLK_EN		(1 << 9)
> > > +	#define SPEAR1340_PCIE_CFG_AUX_CLK_EN		(1 << 8)
> > > +	#define SPEAR1340_SATA_CFG_TX_CLK_EN		(1 << 4)
> > > +	#define SPEAR1340_SATA_CFG_RX_CLK_EN		(1 << 3)
> > > +	#define SPEAR1340_SATA_CFG_POWERUP_RESET	(1 << 2)
> > > +	#define SPEAR1340_SATA_CFG_PM_CLK_EN		(1 << 1)
> > > +	#define SPEAR1340_PCIE_SATA_SEL_PCIE		(0)
> > > +	#define SPEAR1340_PCIE_SATA_SEL_SATA		(1)
> > > +	#define SPEAR1340_PCIE_SATA_CFG_MASK		0xF1F
> > > +	#define SPEAR1340_PCIE_CFG_VAL	(SPEAR1340_PCIE_SATA_SEL_PCIE | \
> > > +			SPEAR1340_PCIE_CFG_AUX_CLK_EN | \
> > > +			SPEAR1340_PCIE_CFG_CORE_CLK_EN | \
> > > +			SPEAR1340_PCIE_CFG_POWERUP_RESET | \
> > > +			SPEAR1340_PCIE_CFG_DEVICE_PRESENT)
> > > +	#define SPEAR1340_SATA_CFG_VAL	(SPEAR1340_PCIE_SATA_SEL_SATA | \
> > > +			SPEAR1340_SATA_CFG_PM_CLK_EN | \
> > > +			SPEAR1340_SATA_CFG_POWERUP_RESET | \
> > > +			SPEAR1340_SATA_CFG_RX_CLK_EN | \
> > > +			SPEAR1340_SATA_CFG_TX_CLK_EN)
> > > +
> > > +#define SPEAR1340_PCIE_MIPHY_CFG		0x428
> > > +	#define SPEAR1340_MIPHY_OSC_BYPASS_EXT		(1 << 31)
> > > +	#define SPEAR1340_MIPHY_CLK_REF_DIV2		(1 << 27)
> > > +	#define SPEAR1340_MIPHY_CLK_REF_DIV4		(2 << 27)
> > > +	#define SPEAR1340_MIPHY_CLK_REF_DIV8		(3 << 27)
> > > +	#define SPEAR1340_MIPHY_PLL_RATIO_TOP(x)	(x << 0)
> > > +	#define SPEAR1340_PCIE_MIPHY_CFG_MASK		0xF80000FF
> > > +	#define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA \
> > > +			(SPEAR1340_MIPHY_OSC_BYPASS_EXT | \
> > > +			SPEAR1340_MIPHY_CLK_REF_DIV2 | \
> > > +			SPEAR1340_MIPHY_PLL_RATIO_TOP(60))
> > > +	#define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK \
> > > +			(SPEAR1340_MIPHY_PLL_RATIO_TOP(120))
> > > +	#define SPEAR1340_PCIE_SATA_MIPHY_CFG_PCIE \
> > > +			(SPEAR1340_MIPHY_OSC_BYPASS_EXT | \
> > > +			SPEAR1340_MIPHY_PLL_RATIO_TOP(25))
> > > +
> > > +struct spear13xx_cfg_priv {
> > > +	struct regmap		*misc;
> > > +};
> > > +
> > > +/* SATA device registration */
> > > +static void spear1340_sata_miphy_init(struct spear13xx_cfg_priv *cfgpriv)
> > > +{
> > > +	regmap_update_bits(cfgpriv->misc, SPEAR1340_PCIE_SATA_CFG,
> > > +			SPEAR1340_PCIE_SATA_CFG_MASK, SPEAR1340_SATA_CFG_VAL);
> > > +	regmap_update_bits(cfgpriv->misc, SPEAR1340_PCIE_MIPHY_CFG,
> > > +			SPEAR1340_PCIE_MIPHY_CFG_MASK,
> > > +			SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK);
> > > +	/* Switch on sata power domain */
> > > +	regmap_update_bits(cfgpriv->misc, SPEAR1340_PCM_CFG,
> > > +			SPEAR1340_PCM_CFG_SATA_POWER_EN,
> > > +			SPEAR1340_PCM_CFG_SATA_POWER_EN);
> > > +	msleep(20);
> > > +	/* Disable PCIE SATA Controller reset */
> > > +	regmap_update_bits(cfgpriv->misc, SPEAR1340_PERIP1_SW_RST,
> > > +			SPEAR1340_PERIP1_SW_RST_SATA, 0);
> > > +	msleep(20);
> > > +}
> > 
> > Looking at the actual code now, this very much looks like it ought to
> > be a "phy" driver and get put in drivers/phy/.
> 
> Actually these registers are part of common system configurations
> register space (called as misc space) for SPEAr SOC. So we opted for
> syscon framework.

The use of syscon for this is good, I have no objection to that, and
was suggesting that you create a logical "phy" device that uses the
misc syscon device as a backend.
 
> PHY registers space starts from 0xEB800000, which can be
> programmed for various phy specific functions like power management,
> tx/rx settings, comparator settings etc. In most of the cases phy
> works with default settings, however there are few exceptions for
> which we will be adding a phy driver for further improvement of SPEAr
> drivers.

I see. So while the code you have here could be expressed as a phy driver
by itself, there is another part of the SoC that controls the actual
phy. How about if you add the phy device node to DT, and write a driver
that doesn't actually program the phy registers for now, but does contain
the code that you have posted here. That would give you flexibility for
future extensions and at the same time let you remove all SPEAr specific
code from the actual AHCI driver by using the generic ahci-platform
driver.

	Arnd

^ 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