Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/18] regulator: max14577: Remove unused state container definition
From: Mark Brown @ 2014-01-28 16:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390911522-28209-2-git-send-email-k.kozlowski@samsung.com>

On Tue, Jan 28, 2014 at 01:18:25PM +0100, Krzysztof Kozlowski wrote:
> Remove the "struct max14577_regulator" because this is not used
> anywhere. It should be removed earlier along with changing the driver
> after review on the mailing lists.

Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140128/f46a547b/attachment.sig>

^ permalink raw reply

* [PATCH V2] arm64: add DSB after icache flush in __flush_icache_all()
From: Will Deacon @ 2014-01-28 16:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390892813-30407-1-git-send-email-vkale@apm.com>

On Tue, Jan 28, 2014 at 07:06:53AM +0000, Vinayak Kale wrote:
> Add DSB after icache flush. It's needed to complete the cache maintenance
> operation. The function __flush_icache_all() is used only for user space
> mappings and an ISB is not required because of an exception return before
> executing user instructions. An exception return would behave like an ISB.
> 
> This patch also uses 'memory' clobber for flush operation instruction to 
> prevent instruction re-ordering by compiler. 
> 
> Signed-off-by: Vinayak Kale <vkale@apm.com>
> ---
> 
> V2: - Add more desciption in the commit message as suggested by Catalin & Will
>     - Use 'memory' clobber for flush instruction as suggested by Will

Please can you check and fix other occurrences of this bug too, as I asked
in v1? For example, a 2 second grep shows problems with data-cache
maintenance in kvm. I can also see the same problem for system register
writes followed up with isb.

I also don't buy you not being able to test AArch32 kernels. Does KVM not
work for you?

Will

> 
>  arch/arm64/include/asm/cacheflush.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h
> index fea9ee3..bd30f42 100644
> --- a/arch/arm64/include/asm/cacheflush.h
> +++ b/arch/arm64/include/asm/cacheflush.h
> @@ -115,7 +115,8 @@ extern void flush_dcache_page(struct page *);
>  
>  static inline void __flush_icache_all(void)
>  {
> -	asm("ic	ialluis");
> +	asm volatile("ic ialluis" : : : "memory");
> +	dsb();
>  }
>  
>  #define flush_dcache_mmap_lock(mapping) \
> -- 
> 1.8.2.1
> 
> 

^ permalink raw reply

* [PATCH] ARM: multi_v7: add mvebu and drivers to defconfig
From: Kevin Hilman @ 2014-01-28 15:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52E7CF49.8040703@free-electrons.com>

On Tue, Jan 28, 2014 at 7:39 AM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> On 28/01/2014 16:17, Kevin Hilman wrote:
>> Jason Cooper <jason@lakedaemon.net> writes:
>>
>>> boot farms testing is highlighting some errors in mvebu.  Let's get some
>>> more coverage for multi_v7_defconfig kernels.
>>>
>>> Signed-off-by: Jason Cooper <jason@lakedaemon.net>
>>> ---
>>> Kevin,
>>>
>>> This is against tonight's master.  Feel free to take it directly.
>>>
>>
>> OK, but this one doesn't currently fix any known errors, right?  (the
>> pull request you sent yesterday fixes the boot regressions, IIUC.)  The
>> goal here is just to get more mvebu coverage in multi_v7_defconfig,
>> correct?
>>
>> That being said, have a closer look at the Kconfig entries that are
>> removed by the patch below, many of which will have an impact on other
>> platforms.
>
> Hi Kevin,
>
> I may be wrong but I suspect this config file was generated with make savedefconfig.
> this rule only generate the CONFIG that are not implied by other CONFIG,
> so I wonder if all this changes are just due to of modificatino in the Kconfig
> themselves.

Yes, I suspect that as well, which means the changelog needs to have a
bit more detail justifying those removals that look suspicious when
just looking through the diff.

Kevin

^ permalink raw reply

* [PATCH 2/2] arm64: kernel: use seq_puts() instead of seq_printf()
From: Catalin Marinas @ 2014-01-28 15:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <001f01cf1bc9$57767070$06635150$%han@samsung.com>

On Tue, Jan 28, 2014 at 01:36:18AM +0000, Jingoo Han wrote:
> For a constant format without additional arguments, use seq_puts()
> instead of seq_printf(). Also, it fixes the following checkpatch
> warning.
> 
>   WARNING: Prefer seq_puts to seq_printf
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  arch/arm64/kernel/setup.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index c8e9eff..4507691 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -416,7 +416,7 @@ static int c_show(struct seq_file *m, void *v)
>  			seq_printf(m, "%s ", hwcap_str[i]);
>  
>  	seq_printf(m, "\nCPU implementer\t: 0x%02x\n", read_cpuid_id() >> 24);
> -	seq_printf(m, "CPU architecture: AArch64\n");
> +	seq_puts(m, "CPU architecture: AArch64\n");
>  	seq_printf(m, "CPU variant\t: 0x%x\n", (read_cpuid_id() >> 20) & 15);
>  	seq_printf(m, "CPU part\t: 0x%03x\n", (read_cpuid_id() >> 4) & 0xfff);
>  	seq_printf(m, "CPU revision\t: %d\n", read_cpuid_id() & 15);

Just ignore the checkpatch warning. I prefer the consistency of
seq_printf() in this function.

-- 
Catalin

^ permalink raw reply

* [PATCH] ARM: multi_v7: add mvebu and drivers to defconfig
From: Gregory CLEMENT @ 2014-01-28 15:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8761p4w2yu.fsf@paris.lan>

On 28/01/2014 16:17, Kevin Hilman wrote:
> Jason Cooper <jason@lakedaemon.net> writes:
> 
>> boot farms testing is highlighting some errors in mvebu.  Let's get some
>> more coverage for multi_v7_defconfig kernels.
>>
>> Signed-off-by: Jason Cooper <jason@lakedaemon.net>
>> ---
>> Kevin,
>>
>> This is against tonight's master.  Feel free to take it directly.
>>
> 
> OK, but this one doesn't currently fix any known errors, right?  (the
> pull request you sent yesterday fixes the boot regressions, IIUC.)  The
> goal here is just to get more mvebu coverage in multi_v7_defconfig,
> correct?
> 
> That being said, have a closer look at the Kconfig entries that are
> removed by the patch below, many of which will have an impact on other
> platforms.

Hi Kevin,

I may be wrong but I suspect this config file was generated with make savedefconfig.
this rule only generate the CONFIG that are not implied by other CONFIG,
so I wonder if all this changes are just due to of modificatino in the Kconfig
themselves.

For example the config symbol TEGRA_PCI have been removed since v3.12.

Gregory






> 
> Kevin
> 
>>  arch/arm/configs/multi_v7_defconfig | 28 +++++++++++++++++-----------
>>  1 file changed, 17 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
>> index 687e4e811b2a..13e7d649230b 100644
>> --- a/arch/arm/configs/multi_v7_defconfig
>> +++ b/arch/arm/configs/multi_v7_defconfig
>> @@ -38,7 +38,6 @@ CONFIG_ARCH_TEGRA=y
>>  CONFIG_ARCH_TEGRA_2x_SOC=y
>>  CONFIG_ARCH_TEGRA_3x_SOC=y
>>  CONFIG_ARCH_TEGRA_114_SOC=y
>> -CONFIG_TEGRA_PCI=y
> 
> huh?
> 
>>  CONFIG_TEGRA_EMC_SCALING_ENABLE=y
>>  CONFIG_ARCH_U8500=y
>>  CONFIG_MACH_HREFV60=y
>> @@ -49,7 +48,9 @@ CONFIG_ARCH_VEXPRESS_CA9X4=y
>>  CONFIG_ARCH_VIRT=y
>>  CONFIG_ARCH_WM8850=y
>>  CONFIG_ARCH_ZYNQ=y
>> -CONFIG_SMP=y
> 
> huh?
> 
>> +CONFIG_PCI=y
>> +CONFIG_PCI_MSI=y
>> +CONFIG_PCI_MVEBU=y
>>  CONFIG_HIGHPTE=y
>>  CONFIG_ARM_APPENDED_DTB=y
>>  CONFIG_ARM_ATAG_DTB_COMPAT=y
>> @@ -69,12 +70,14 @@ CONFIG_SATA_MV=y
>>  CONFIG_NETDEVICES=y
>>  CONFIG_SUN4I_EMAC=y
>>  CONFIG_NET_CALXEDA_XGMAC=y
>> +CONFIG_MVNETA=y
>>  CONFIG_KS8851=y
>>  CONFIG_SMSC911X=y
>>  CONFIG_STMMAC_ETH=y
>> -CONFIG_ICPLUS_PHY=y
>> -CONFIG_MDIO_SUN4I=y
>>  CONFIG_TI_CPSW=y
>> +CONFIG_MARVELL_PHY=y
>> +CONFIG_ICPLUS_PHY=y
>> +CONFIG_KEYBOARD_GPIO=y
>>  CONFIG_KEYBOARD_SPEAR=y
>>  CONFIG_SERIO_AMBAKMI=y
>>  CONFIG_SERIAL_8250=y
>> @@ -99,25 +102,27 @@ CONFIG_SERIAL_FSL_LPUART_CONSOLE=y
>>  CONFIG_SERIAL_ST_ASC=y
>>  CONFIG_SERIAL_ST_ASC_CONSOLE=y
>>  CONFIG_I2C_DESIGNWARE_PLATFORM=y
>> +CONFIG_I2C_MV64XXX=y
>>  CONFIG_I2C_SIRF=y
>>  CONFIG_I2C_TEGRA=y
>>  CONFIG_SPI=y
>>  CONFIG_SPI_OMAP24XX=y
>> +CONFIG_SPI_ORION=y
>>  CONFIG_SPI_PL022=y
>>  CONFIG_SPI_SIRF=y
>>  CONFIG_SPI_TEGRA114=y
>>  CONFIG_SPI_TEGRA20_SLINK=y
>> -CONFIG_PINCTRL_SINGLE=y
>>  CONFIG_GPIO_GENERIC_PLATFORM=y
>>  CONFIG_GPIO_TWL4030=y
>> -CONFIG_REGULATOR_GPIO=y
>> +CONFIG_THERMAL=y
>> +CONFIG_ARMADA_THERMAL=y
>>  CONFIG_REGULATOR_AB8500=y
>> +CONFIG_REGULATOR_GPIO=y
>>  CONFIG_REGULATOR_TPS51632=y
>>  CONFIG_REGULATOR_TPS62360=y
>>  CONFIG_REGULATOR_TWL4030=y
>>  CONFIG_REGULATOR_VEXPRESS=y
>>  CONFIG_DRM=y
>> -CONFIG_TEGRA_HOST1X=y
> 
> huh?
> 
>>  CONFIG_DRM_TEGRA=y
>>  CONFIG_FB_ARMCLCD=y
>>  CONFIG_FB_WM8505=y
>> @@ -132,8 +137,6 @@ CONFIG_USB_STORAGE=y
>>  CONFIG_USB_CHIPIDEA=y
>>  CONFIG_USB_CHIPIDEA_HOST=y
>>  CONFIG_AB8500_USB=y
>> -CONFIG_NOP_USB_XCEIV=y
>> -CONFIG_OMAP_USB2=y
> 
> huh?
> 
>>  CONFIG_OMAP_USB3=y
>>  CONFIG_SAMSUNG_USB2PHY=y
>>  CONFIG_SAMSUNG_USB3PHY=y
>> @@ -144,13 +147,13 @@ CONFIG_MMC=y
>>  CONFIG_MMC_BLOCK_MINORS=16
>>  CONFIG_MMC_ARMMMCI=y
>>  CONFIG_MMC_SDHCI=y
>> -CONFIG_MMC_SDHCI_PLTFM=y
>>  CONFIG_MMC_SDHCI_ESDHC_IMX=y
>>  CONFIG_MMC_SDHCI_TEGRA=y
>>  CONFIG_MMC_SDHCI_SPEAR=y
>>  CONFIG_MMC_SDHCI_BCM_KONA=y
>>  CONFIG_MMC_OMAP=y
>>  CONFIG_MMC_OMAP_HS=y
>> +CONFIG_MMC_MVSDIO=y
>>  CONFIG_EDAC=y
>>  CONFIG_EDAC_MM_EDAC=y
>>  CONFIG_EDAC_HIGHBANK_MC=y
>> @@ -159,20 +162,23 @@ CONFIG_RTC_CLASS=y
>>  CONFIG_RTC_DRV_TWL4030=y
>>  CONFIG_RTC_DRV_PL031=y
>>  CONFIG_RTC_DRV_VT8500=y
>> +CONFIG_RTC_DRV_MV=y
>>  CONFIG_RTC_DRV_TEGRA=y
>>  CONFIG_DMADEVICES=y
>>  CONFIG_DW_DMAC=y
>> +CONFIG_MV_XOR=y
>>  CONFIG_TEGRA20_APB_DMA=y
>>  CONFIG_STE_DMA40=y
>>  CONFIG_SIRF_DMA=y
>> -CONFIG_TI_EDMA=y
>>  CONFIG_PL330_DMA=y
>>  CONFIG_IMX_SDMA=y
>>  CONFIG_IMX_DMA=y
>>  CONFIG_MXS_DMA=y
>>  CONFIG_DMA_OMAP=y
>> +CONFIG_MEMORY=y
>>  CONFIG_PWM=y
>>  CONFIG_PWM_VT8500=y
>> +CONFIG_OMAP_USB2=y
>>  CONFIG_EXT4_FS=y
>>  CONFIG_TMPFS=y
>>  CONFIG_NFS_FS=y


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
From: Heikki Krogerus @ 2014-01-28 15:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140127160458.GF13268@saruman.home>

Hi,

On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote:
> > Why would you need to know if the PHY drivers are needed or not
> > explicitly in your controller driver?
> 
> because, one way or another, they all do need it. Except for quirky ones
> like AM437x where a USB3 IP was hardwired into USB2-only mode, so it
> really lacks a USB3 PHY.

The Baytrail board I deal with has completely autonomous PHYs. But my
question is, why would you need to care about the PHYs in your
controller driver? Why can't you leave the responsibility of them to
the upper layers and trust what they tell you?

If there is no USB3 PHY for dwc3 then, the driver gets something like
-ENODEV and just continues. There is no need to know about the
details.

For the controller drivers the PHYs are just a resource like any
other. The controller drivers can't have any responsibility of
them. They should not care if PHY drivers are available for them or
not, or even if the PHY framework is available or not.

> > > But I really want to see the argument against using no-op. As far as I
> > > could see, everybody needs a PHY driver one way or another, some
> > > platforms just haven't sent any PHY driver upstream and have their own
> > > hacked up solution to avoid using the PHY layer.
> > 
> > Not true in our case. Platforms using Intel's SoCs and chip sets may
> > or may not have controllable USB PHY. Quite often they don't. The
> > Baytrails have usually ULPI PHY for USB2, but that does not mean they
> > provide any vendor specific functions or any need for a driver in any
> > case.
> 
> that's different from what I heard.

I don't know where you got that impression, but it's not true. The
Baytrail SoCs for example don't have internal USB PHYs, which means
the manufacturers using it can select what they want. So we have
boards where PHY driver(s) is needed and boards where it isn't.

The problem is that we just don't always know all the details about
the platform. If the PHY is ULPI PHY, we can do runtime detection, but
we can't even rely on always having ULPI.

> > Are we talking about the old USB PHY library or the new PHY framework
> > with the no-op PHY driver?
> > 
> > Well, in any case, I don't understand what is the purpose of the no-op
> > PHY driver. What are you drying to achieve with that?
> 
> I'm trying to avoid supporting 500 different combinations for a single
> driver. We already support old USB PHY layer and generic PHY layer, now
> they both need to be made optional.

This is really good to get. We have some projects where we are dealing
with more embedded environments, like IVI, where the kernel should be
stripped of everything useless. Since the PHYs are autonomous, we
should be able to disable the PHY libraries/frameworks.

But I still don't understand what is the benefit in the no-op? You
really need to explain this. What you have now in dwc3 is expectations
regarding the PHY, which put a lot of pressure on upper layers to
satisfy the driver. The no-op is just an extra thing that you have to
provide when you fail to determine if the system requires a PHY driver
or not, or if you know that it doesn't, plus an additional check for
the case where the PHY lib/framework is not enabled. I don't see the
value in it.

> The old USB PHY layer also provides
> a no-op, now called phy-generic, which is actually pretty useful.
> 
> On top of all that, I'm sure you'll subscribe to the idea of preventing
> dwc3 from becoming another MUSB which supports several different
> configurations and none work correctly. I much prefer to take baby steps
> which are well thought-out and very well exercised, so I'll be very
> pedantic about proof of testing.

I think our goals are the same. I just want to also minimize the need
for any platform specific extra work from the upper layers regarding
the PHYs.


Thanks,

-- 
heikki

^ permalink raw reply

* [RFC] drivers: irq-chip: Enable SGI's for Secure-to-NonSecure communication use-cases
From: Bill Pringlemeir @ 2014-01-28 15:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <79CD15C6BA57404B839C016229A409A83EDC1E71@DBDE04.ent.ti.com>


>>> You have any other alternative?

>> From: Bill Pringlemeir [mailto:bpringlemeir at nbsps.com]

>> I think you need to put Bhupesh Sharma's comment with this.  The
>> typical sane mode for GIC with TZ is to have the monitor mode toggle
>> the IRQ/FIQ routing bits in the SCR (cp15) bits 1,2.  That is, the
>> IRQ goes direct to core and FIQ goes to monitor from the 'Normal'
>> world.  In the 'Secure' world, the FIQ goes direct to core and IRQ
>> traps to monitor.  The monitor mode vector table has a gateway from
>> secure to normal for IRQ and gateway from normal to secure for FIQ.
>>
>> Now, consider the 'SMC' instruction and what is present in stuff like
>> this,
>>
>> http://lwn.net/Articles/513756/

>> Instead of messing around with the GIC, why not use something even
>> more generic like the 'SMC' instruction.  It has the same sort of
>> 'end game' which is a trap to monitor mode.  The monitor has to be a
>> little smarter to determine which world called but this should always
>> be the case; really you want to check this.
>>
>> Btw, the situation is the same no matter which world Linux is in.  I
>> don't think Linux can be the recipient of an 'SMC' call.  But I think
>> most use cases would put it in the 'normal world' and the SMC is
>> fine.

On 28 Jan 2014, hvaibhav at ti.com wrote:

> May be I am missing something here,

> I find your above two statements contradictory,

> If we want to use SMC as you mentioned, and assuming Secure Monitor
> mode is intelligent enough to determine the calling world (whether
> secure or non-secure), then without Linux being recipient (in any
> world) of an 'SMC' call how can realtime switch possible from secure
> world to non-secure world??

I don't think that using an SMC in either world is any different than
writing to the GIC distributors SGIR?  So, the generation is the same.
The monitor switching should be the same.  The only difference is where
does the world switch ends up.

> Just to clarify,

> The need here is, to switch from secure world to non-secure world on
> any realtime (multiple) hardware events, which in turn gets
> processed/handled in non-secure world.  In certain cases even we do
> not want non-secure world know about the hardware event. In this case,
> the Processing of hardware event completely happens in secure world,
> and different event/trigger/info/message goes to non-secure world.

Ok.  Your needs are backwards to my understanding.  You want the
'secure/non-linux' to send an SGI to the Linux.  Ie, you want the
recipient case for the normal world.  The normal world can always issue
an SMC during boot to register a recipient mechanism.  You can fake
another interrupt chip or use some other mechanism.

> So just manipulating IRQ/FIQ routing will not solve the need here. :)

I didn't mean this would solve your problem.  I meant this keeps the
latency to a minimum when using TrustZone.  If an 'interrupt' (FIQ or
IRQ) source occurs in the destination world, then the path is as per
usual.  It is only when an 'interrupt' occurs in the opposite world when
a world switch is needed; this is a fairly expensive operation.

In your case, it sounds like you want the secure world to handle all
interrupts or at least the majority.  This has a pre-defined destination
in a jump to the vector table of the opposite world.  This is the
'recipient' mechanism.  At the very least a 'multi-chip' IRQ could be
used and you can ldrex/strex with world shareable memory to communicate
interrupt sources to the other world; the world shareable memory acts as
a 'IRQ controller register set'.  With the SMC, you have the opportunity
to transfer some information in registers.  It has a bigger question of
how the schedulers would inter-act.  Do you have a scheduler in the
secure world?

I think these are 'alternatives' as you asked?

Regards,
Bill Pringlemeir.

^ permalink raw reply

* [PATCH] ARM: multi_v7: add mvebu and drivers to defconfig
From: Kevin Hilman @ 2014-01-28 15:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390871129-4838-1-git-send-email-jason@lakedaemon.net>

Jason Cooper <jason@lakedaemon.net> writes:

> boot farms testing is highlighting some errors in mvebu.  Let's get some
> more coverage for multi_v7_defconfig kernels.
>
> Signed-off-by: Jason Cooper <jason@lakedaemon.net>
> ---
> Kevin,
>
> This is against tonight's master.  Feel free to take it directly.
>

OK, but this one doesn't currently fix any known errors, right?  (the
pull request you sent yesterday fixes the boot regressions, IIUC.)  The
goal here is just to get more mvebu coverage in multi_v7_defconfig,
correct?

That being said, have a closer look at the Kconfig entries that are
removed by the patch below, many of which will have an impact on other
platforms.

Kevin

>  arch/arm/configs/multi_v7_defconfig | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index 687e4e811b2a..13e7d649230b 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -38,7 +38,6 @@ CONFIG_ARCH_TEGRA=y
>  CONFIG_ARCH_TEGRA_2x_SOC=y
>  CONFIG_ARCH_TEGRA_3x_SOC=y
>  CONFIG_ARCH_TEGRA_114_SOC=y
> -CONFIG_TEGRA_PCI=y

huh?

>  CONFIG_TEGRA_EMC_SCALING_ENABLE=y
>  CONFIG_ARCH_U8500=y
>  CONFIG_MACH_HREFV60=y
> @@ -49,7 +48,9 @@ CONFIG_ARCH_VEXPRESS_CA9X4=y
>  CONFIG_ARCH_VIRT=y
>  CONFIG_ARCH_WM8850=y
>  CONFIG_ARCH_ZYNQ=y
> -CONFIG_SMP=y

huh?

> +CONFIG_PCI=y
> +CONFIG_PCI_MSI=y
> +CONFIG_PCI_MVEBU=y
>  CONFIG_HIGHPTE=y
>  CONFIG_ARM_APPENDED_DTB=y
>  CONFIG_ARM_ATAG_DTB_COMPAT=y
> @@ -69,12 +70,14 @@ CONFIG_SATA_MV=y
>  CONFIG_NETDEVICES=y
>  CONFIG_SUN4I_EMAC=y
>  CONFIG_NET_CALXEDA_XGMAC=y
> +CONFIG_MVNETA=y
>  CONFIG_KS8851=y
>  CONFIG_SMSC911X=y
>  CONFIG_STMMAC_ETH=y
> -CONFIG_ICPLUS_PHY=y
> -CONFIG_MDIO_SUN4I=y
>  CONFIG_TI_CPSW=y
> +CONFIG_MARVELL_PHY=y
> +CONFIG_ICPLUS_PHY=y
> +CONFIG_KEYBOARD_GPIO=y
>  CONFIG_KEYBOARD_SPEAR=y
>  CONFIG_SERIO_AMBAKMI=y
>  CONFIG_SERIAL_8250=y
> @@ -99,25 +102,27 @@ CONFIG_SERIAL_FSL_LPUART_CONSOLE=y
>  CONFIG_SERIAL_ST_ASC=y
>  CONFIG_SERIAL_ST_ASC_CONSOLE=y
>  CONFIG_I2C_DESIGNWARE_PLATFORM=y
> +CONFIG_I2C_MV64XXX=y
>  CONFIG_I2C_SIRF=y
>  CONFIG_I2C_TEGRA=y
>  CONFIG_SPI=y
>  CONFIG_SPI_OMAP24XX=y
> +CONFIG_SPI_ORION=y
>  CONFIG_SPI_PL022=y
>  CONFIG_SPI_SIRF=y
>  CONFIG_SPI_TEGRA114=y
>  CONFIG_SPI_TEGRA20_SLINK=y
> -CONFIG_PINCTRL_SINGLE=y
>  CONFIG_GPIO_GENERIC_PLATFORM=y
>  CONFIG_GPIO_TWL4030=y
> -CONFIG_REGULATOR_GPIO=y
> +CONFIG_THERMAL=y
> +CONFIG_ARMADA_THERMAL=y
>  CONFIG_REGULATOR_AB8500=y
> +CONFIG_REGULATOR_GPIO=y
>  CONFIG_REGULATOR_TPS51632=y
>  CONFIG_REGULATOR_TPS62360=y
>  CONFIG_REGULATOR_TWL4030=y
>  CONFIG_REGULATOR_VEXPRESS=y
>  CONFIG_DRM=y
> -CONFIG_TEGRA_HOST1X=y

huh?

>  CONFIG_DRM_TEGRA=y
>  CONFIG_FB_ARMCLCD=y
>  CONFIG_FB_WM8505=y
> @@ -132,8 +137,6 @@ CONFIG_USB_STORAGE=y
>  CONFIG_USB_CHIPIDEA=y
>  CONFIG_USB_CHIPIDEA_HOST=y
>  CONFIG_AB8500_USB=y
> -CONFIG_NOP_USB_XCEIV=y
> -CONFIG_OMAP_USB2=y

huh?

>  CONFIG_OMAP_USB3=y
>  CONFIG_SAMSUNG_USB2PHY=y
>  CONFIG_SAMSUNG_USB3PHY=y
> @@ -144,13 +147,13 @@ CONFIG_MMC=y
>  CONFIG_MMC_BLOCK_MINORS=16
>  CONFIG_MMC_ARMMMCI=y
>  CONFIG_MMC_SDHCI=y
> -CONFIG_MMC_SDHCI_PLTFM=y
>  CONFIG_MMC_SDHCI_ESDHC_IMX=y
>  CONFIG_MMC_SDHCI_TEGRA=y
>  CONFIG_MMC_SDHCI_SPEAR=y
>  CONFIG_MMC_SDHCI_BCM_KONA=y
>  CONFIG_MMC_OMAP=y
>  CONFIG_MMC_OMAP_HS=y
> +CONFIG_MMC_MVSDIO=y
>  CONFIG_EDAC=y
>  CONFIG_EDAC_MM_EDAC=y
>  CONFIG_EDAC_HIGHBANK_MC=y
> @@ -159,20 +162,23 @@ CONFIG_RTC_CLASS=y
>  CONFIG_RTC_DRV_TWL4030=y
>  CONFIG_RTC_DRV_PL031=y
>  CONFIG_RTC_DRV_VT8500=y
> +CONFIG_RTC_DRV_MV=y
>  CONFIG_RTC_DRV_TEGRA=y
>  CONFIG_DMADEVICES=y
>  CONFIG_DW_DMAC=y
> +CONFIG_MV_XOR=y
>  CONFIG_TEGRA20_APB_DMA=y
>  CONFIG_STE_DMA40=y
>  CONFIG_SIRF_DMA=y
> -CONFIG_TI_EDMA=y
>  CONFIG_PL330_DMA=y
>  CONFIG_IMX_SDMA=y
>  CONFIG_IMX_DMA=y
>  CONFIG_MXS_DMA=y
>  CONFIG_DMA_OMAP=y
> +CONFIG_MEMORY=y
>  CONFIG_PWM=y
>  CONFIG_PWM_VT8500=y
> +CONFIG_OMAP_USB2=y
>  CONFIG_EXT4_FS=y
>  CONFIG_TMPFS=y
>  CONFIG_NFS_FS=y

^ permalink raw reply

* [PATCH v2 1/7] cpufreq: cpufreq-cpu0: allow optional safe voltage during frequency transitions
From: Thomas Abraham @ 2014-01-28 15:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140128160633.470202ec@amdc2363>

Hi Lukasz,

On Tue, Jan 28, 2014 at 8:36 PM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> Hi Thomas,
>
>> Hi Lukasz,
>>
>> On Tue, Jan 28, 2014 at 1:47 PM, Lukasz Majewski
>> <l.majewski@samsung.com> wrote:
>> > Hi Thomas, Mike
>> >
>> >> Hi Mike,
>> >>
>> >> On Tue, Jan 28, 2014 at 1:55 AM, Mike Turquette
>> >> <mturquette@linaro.org> wrote:
>> >> > Quoting Thomas Abraham (2014-01-18 04:10:51)
>> >> >> From: Thomas Abraham <thomas.ab@samsung.com>
>> >> >>
>> >> >> On some platforms such as the Samsung Exynos, changing the
>> >> >> frequency of the CPU clock requires changing the frequency of
>> >> >> the PLL that is supplying the CPU clock. To change the
>> >> >> frequency of the PLL, the CPU clock is temporarily reparented
>> >> >> to another parent clock.
>> >> >>
>> >> >> The clock frequency of this temporary parent clock could be much
>> >> >> higher than the clock frequency of the PLL at the time of
>> >> >> reparenting. Due to the temporary increase in the CPU clock
>> >> >> speed, the CPU (and any other components in the CPU clock
>> >> >> domain such as dividers, mux, etc.) have to to be operated at a
>> >> >> higher voltage level, called the safe voltage level. This patch
>> >> >> adds optional support to temporarily switch to a safe voltage
>> >> >> level during CPU frequency transitions.
>> >> >>
>> >> >> Cc: Shawn Guo <shawn.guo@linaro.org>
>> >> >> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>> >> >
>> >> > I'm not a fan of this change. This corner case should be
>> >> > abstracted away somehow. I had talked to Chander Kayshap
>> >> > previously about handling voltage changes in clock notifier
>> >> > callbacks, which then renders any voltage change as a trivial
>> >> > part of the clock rate transition. That means that this "safe
>> >> > voltage" thing could be handled automagically without any
>> >> > additional code in the CPUfreq driver.
>> >> >
>> >> > There are two nice ways to do this with the clock framework.
>> >> > First is explicit re-parenting with voltage scaling done in the
>> >> > clock rate-change notifiers:
>> >> >
>> >> > clk_set_parent(cpu_clk, temp_parent);
>> >> > /* implicit voltage scaling to "safe voltage" happens above */
>> >> > clk_set_rate(pll, some_rate);
>> >> > clk_set_parent(cpu_clk, pll);
>> >> > /* implicit voltage scaling to nominal OPP voltage happens above
>> >> > */
>> >> >
>> >
>> > I must agree with Mike here. In my opinion the above approach is
>> > more compliant with CCF (as I've pointed it out in my other comment
>> > - the cpu_clk has more than one parent and we could switch between
>> > them when needed).
>>
>> The mux which is used to re-parent is part of the larger opaque cpu
>> clock type (more like the composite clock). So I am not sure how this
>> isn't compliant with ccf.
>
> My point here is to use the clk_set_parent() explicitly instead of
> changing the mux with writing values directly to registers.
>
> However, I'm also aware, that we must reparent quickly. so I'm OK
> with your approach.

Okay.

>
>>
>> >
>> >> > The above sequence would require a separate exnyos CPUfreq
>> >> > driver, due to the added clk_set_parent logic.
>> >> >
>> >> > The second way to do this is to abstract the clk re-muxing logic
>> >> > out into the clk driver, which would allow cpufreq-cpu0 to be
>> >> > used for the exynos chips.
>> >>
>> >> This is the approach this patch series takes (patch 2/7). The clock
>> >> re-muxing logic is handled by a clock driver code. The difference
>> >> from what you suggested is that the safe voltage (that may be
>> >> optionally) required before doing the re-muxing is handled here in
>> >> cpufreq-cpu0 driver.
>> >>
>> >> The safe voltage setup can be done in the notifier as you
>> >> suggested.
>> >
>> > If the clk_set_parent() approach is not suitable, then cannot we
>> > consider using the one from highbank-cpufreq.c?
>> >
>> > Here we have cpufreq-cpu0.c which sets voltage of the cpu_clk.
>> > In the highbank-cpufreq.c there are clock notifiers to change the
>> > voltage.
>> >
>> > Cannot Exynos reuse such approach? Why shall we pollute
>> > cpufreq-cpu0.c with another solution?
>>
>> The highbank-cpufreq.c file was introduced because platforms using
>> this driver did not have the usual regulator to control the voltage.
>> The first commit of this driver explains this (copied below).
>>
>> "Highbank processors depend on the external ECME to perform voltage
>> management based on a requested frequency. Communication between the
>> A9 cores and the ECME happens over the pl320 IPC channel."
>>
>> So those platforms had no choice but to use an alternative approach to
>> control the voltage (and reuse cpufreq-cpu0 as much as possible).
>> The
>> case with exynos is a different one.
>
> Highbank needs to set voltage via IPC, Exynos needs to adjust voltage
> when reparenting.
>
> Both can be recognized as unusual cases. That is why I asked if we
> could reuse the same approach for Exynos.

Okay.

>
>> cpufreq-cpu0 is fully re-usable
>> for exynos with the additional support for "safe voltage". If we agree
>> that there might be existing or future platforms with single
>> clock/voltage rail that require the "safe voltage" feature, then
>> adding "safe voltage" support in cpufreq-cpu0 driver seems to be the
>> right approach.
>
> I think that Shawn's opinion will be final here.
>
>>
>> >
>> >> But, doing that in cpufreq-cpu0 driver will help other platforms
>> >> reuse this feature if required. Also, if done here, the regulator
>> >> handling is localized in this driver which otherwise would need to
>> >> be handled in two places, cpufreq-cpu0 driver and the clock
>> >> notifier.
>> >
>> > I think that there is a logical distinction between setting voltage
>> > for cpufreq-cpu0 related clock and increasing voltage of reparented
>> > clock.
>> >
>> > The former fits naturally to cpufreq-cpu0, when the latter seems
>> > like some corner case (as Mike pointed out) for Exynos.
>>
>> Agreed, it is a corner case. But for this corner case, we are
>> performing some additional actions on the same regulator which is used
>> in the normal functioning of the driver.
>>
>> >
>> >>
>> >> So I tend to prefer the approach in this patch but I am willing to
>> >> consider any suggestions.
>> >
>> > Thomas, what do you think about highbank-cpufreq.c approach (with
>> > using clock notifiers)?
>>
>> I have made a related comment on this above.
>>
>> >
>> > Do you think, that it is feasible to reuse it with Exynos?
>>
>> highbank cpufreq driver is intended for a different purpose so I don't
>> think it can be reused for exynos. Yes, we can make exynos specific
>> hacks into highbank driver but how would that be better over the
>> approach in this patch?
>
> I think, that I was misunderstood here. I wanted to ask if we could
> reuse the clk notifier approach.

Okay, I misunderstood your comment. We could do something similar to
highbank cpufreq driver for exynos as well. Anyways, Shawn prefers not
to add "safe voltage" support into cpufreq-cpu0 driver. So we need to
look for other options.

Thanks,
Thomas.

>
>>
>> >
>> >> Shawn, it would be helpful if you could let
>> >> us know your thoughts on this. I am almost done with testing the
>> >> v3 of this series and want to post it so if there are any
>> >> objections to the changes in this patch, please let me know.
>> >>
>> >> Thanks,
>> >> Thomas.
>> >>
>> >> >
>> >> > I'm more a fan of explicitly listing the Exact Steps for the cpu
>> >> > opp transition in a separate exynos-specific CPUfreq driver, but
>> >> > that's probably an unpopular view.
>> >> >
>> >> > Regards,
>> >> > Mike
>> >> >
>> >> >> ---
>> >> >>  .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |    7 ++++
>> >> >>  drivers/cpufreq/cpufreq-cpu0.c                     |   37
>> >> >> +++++++++++++++++-- 2 files changed, 40 insertions(+), 4
>> >> >> deletions(-)
>> >> >>
>> >> >> diff --git
>> >> >> a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> >> >> b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
>> >> >> index f055515..37453ab 100644 ---
>> >> >> a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt +++
>> >> >> b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt @@
>> >> >> -19,6 +19,12 @@ Optional properties:
>> >> >>  - cooling-min-level:
>> >> >>  - cooling-max-level:
>> >> >>       Please refer to
>> >> >> Documentation/devicetree/bindings/thermal/thermal.txt. +-
>> >> >> safe-opp: Certain platforms require that during a opp
>> >> >> transition,
>> >> >> +  a system should not go below a particular opp level. For such
>> >> >> systems,
>> >> >> +  this property specifies the minimum opp to be maintained
>> >> >> during the
>> >> >> +  opp transitions. The safe-opp value is a tuple with first
>> >> >> element
>> >> >> +  representing the safe frequency and the second element
>> >> >> representing the
>> >> >> +  safe voltage.
>> >> >>
>> >> >>  Examples:
>> >> >>
>> >> >> @@ -36,6 +42,7 @@ cpus {
>> >> >>                         396000  950000
>> >> >>                         198000  850000
>> >> >>                 >;
>> >> >> +               safe-opp = <396000 950000>
>> >> >>                 clock-latency = <61036>; /* two CLK32 periods */
>> >> >>                 #cooling-cells = <2>;
>> >> >>                 cooling-min-level = <0>;
>> >> >> diff --git a/drivers/cpufreq/cpufreq-cpu0.c
>> >> >> b/drivers/cpufreq/cpufreq-cpu0.c index 0c12ffc..075d3d1 100644
>> >> >> --- a/drivers/cpufreq/cpufreq-cpu0.c
>> >> >> +++ b/drivers/cpufreq/cpufreq-cpu0.c
>> >> >> @@ -27,6 +27,8 @@
>> >> >>
>> >> >>  static unsigned int transition_latency;
>> >> >>  static unsigned int voltage_tolerance; /* in percentage */
>> >> >> +static unsigned long safe_frequency;
>> >> >> +static unsigned long safe_voltage;
>> >> >>
>> >> >>  static struct device *cpu_dev;
>> >> >>  static struct clk *cpu_clk;
>> >> >> @@ -64,17 +66,30 @@ static int cpu0_set_target(struct
>> >> >> cpufreq_policy *policy, unsigned int index) volt_old =
>> >> >> regulator_get_voltage(cpu_reg); }
>> >> >>
>> >> >> -       pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
>> >> >> +       pr_debug("\n\n%u MHz, %ld mV --> %u MHz, %ld mV\n",
>> >> >>                  old_freq / 1000, volt_old ? volt_old / 1000 :
>> >> >> -1, new_freq / 1000, volt ? volt / 1000 : -1);
>> >> >>
>> >> >>         /* scaling up?  scale voltage before frequency */
>> >> >> -       if (!IS_ERR(cpu_reg) && new_freq > old_freq) {
>> >> >> +       if (!IS_ERR(cpu_reg) && new_freq > old_freq &&
>> >> >> +                               new_freq >= safe_frequency) {
>> >> >>                 ret = regulator_set_voltage_tol(cpu_reg, volt,
>> >> >> tol); if (ret) {
>> >> >>                         pr_err("failed to scale voltage up:
>> >> >> %d\n", ret); return ret;
>> >> >>                 }
>> >> >> +       } else if (!IS_ERR(cpu_reg) && old_freq <
>> >> >> safe_frequency) {
>> >> >> +               /*
>> >> >> +                * the scaled up voltage level for the new_freq
>> >> >> is lower
>> >> >> +                * than the safe voltage level. so set
>> >> >> safe_voltage
>> >> >> +                * as the intermediate voltage level and revert
>> >> >> it
>> >> >> +                * back after the frequency has been changed.
>> >> >> +                */
>> >> >> +               ret = regulator_set_voltage_tol(cpu_reg,
>> >> >> safe_voltage, tol);
>> >> >> +               if (ret) {
>> >> >> +                       pr_err("failed to set safe voltage:
>> >> >> %d\n", ret);
>> >> >> +                       return ret;
>> >> >> +               }
>> >> >>         }
>> >> >>
>> >> >>         ret = clk_set_rate(cpu_clk, freq_exact);
>> >> >> @@ -86,7 +101,8 @@ static int cpu0_set_target(struct
>> >> >> cpufreq_policy *policy, unsigned int index) }
>> >> >>
>> >> >>         /* scaling down?  scale voltage after frequency */
>> >> >> -       if (!IS_ERR(cpu_reg) && new_freq < old_freq) {
>> >> >> +       if (!IS_ERR(cpu_reg) &&
>> >> >> +                       (new_freq < old_freq || new_freq <
>> >> >> safe_frequency)) { ret = regulator_set_voltage_tol(cpu_reg,
>> >> >> volt, tol); if (ret) {
>> >> >>                         pr_err("failed to scale voltage down:
>> >> >> %d\n", ret); @@ -116,6 +132,8 @@ static struct cpufreq_driver
>> >> >> cpu0_cpufreq_driver = {
>> >> >>
>> >> >>  static int cpu0_cpufreq_probe(struct platform_device *pdev)
>> >> >>  {
>> >> >> +       const struct property *prop;
>> >> >> +       struct dev_pm_opp *opp;
>> >> >>         struct device_node *np;
>> >> >>         int ret;
>> >> >>
>> >> >> @@ -165,13 +183,24 @@ static int cpu0_cpufreq_probe(struct
>> >> >> platform_device *pdev) goto out_put_node;
>> >> >>         }
>> >> >>
>> >> >> +       prop = of_find_property(np, "safe-opp", NULL);
>> >> >> +       if (prop) {
>> >> >> +               if (prop->value && (prop->length / sizeof(u32))
>> >> >> == 2) {
>> >> >> +                       const __be32 *val;
>> >> >> +                       val = prop->value;
>> >> >> +                       safe_frequency = be32_to_cpup(val++);
>> >> >> +                       safe_voltage = be32_to_cpup(val);
>> >> >> +               } else {
>> >> >> +                       pr_err("invalid safe-opp level
>> >> >> specified\n");
>> >> >> +               }
>> >> >> +       }
>> >> >> +
>> >> >>         of_property_read_u32(np, "voltage-tolerance",
>> >> >> &voltage_tolerance);
>> >> >>
>> >> >>         if (of_property_read_u32(np, "clock-latency",
>> >> >> &transition_latency)) transition_latency = CPUFREQ_ETERNAL;
>> >> >>
>> >> >>         if (!IS_ERR(cpu_reg)) {
>> >> >> -               struct dev_pm_opp *opp;
>> >> >>                 unsigned long min_uV, max_uV;
>> >> >>                 int i;
>> >> >>
>> >> >> --
>> >> >> 1.6.6.rc2
>> >> >>
>> >
>> >
>> >
>> > --
>> > Best regards,
>> >
>> > Lukasz Majewski
>> >
>> > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
>>
>> Thanks for your comments Lukasz.
>>
>> Regards,
>> Thomas.
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
>
> --
> Best regards,
>
> Lukasz Majewski
>
> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply

* [PATCH v2 1/7] cpufreq: cpufreq-cpu0: allow optional safe voltage during frequency transitions
From: Lukasz Majewski @ 2014-01-28 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAJuA9ag_izbSmQ_9-NH4NSGeCnzNBdwPUKzBTg5azuFKVqD28g@mail.gmail.com>

Hi Thomas,

> Hi Lukasz,
> 
> On Tue, Jan 28, 2014 at 1:47 PM, Lukasz Majewski
> <l.majewski@samsung.com> wrote:
> > Hi Thomas, Mike
> >
> >> Hi Mike,
> >>
> >> On Tue, Jan 28, 2014 at 1:55 AM, Mike Turquette
> >> <mturquette@linaro.org> wrote:
> >> > Quoting Thomas Abraham (2014-01-18 04:10:51)
> >> >> From: Thomas Abraham <thomas.ab@samsung.com>
> >> >>
> >> >> On some platforms such as the Samsung Exynos, changing the
> >> >> frequency of the CPU clock requires changing the frequency of
> >> >> the PLL that is supplying the CPU clock. To change the
> >> >> frequency of the PLL, the CPU clock is temporarily reparented
> >> >> to another parent clock.
> >> >>
> >> >> The clock frequency of this temporary parent clock could be much
> >> >> higher than the clock frequency of the PLL at the time of
> >> >> reparenting. Due to the temporary increase in the CPU clock
> >> >> speed, the CPU (and any other components in the CPU clock
> >> >> domain such as dividers, mux, etc.) have to to be operated at a
> >> >> higher voltage level, called the safe voltage level. This patch
> >> >> adds optional support to temporarily switch to a safe voltage
> >> >> level during CPU frequency transitions.
> >> >>
> >> >> Cc: Shawn Guo <shawn.guo@linaro.org>
> >> >> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> >> >
> >> > I'm not a fan of this change. This corner case should be
> >> > abstracted away somehow. I had talked to Chander Kayshap
> >> > previously about handling voltage changes in clock notifier
> >> > callbacks, which then renders any voltage change as a trivial
> >> > part of the clock rate transition. That means that this "safe
> >> > voltage" thing could be handled automagically without any
> >> > additional code in the CPUfreq driver.
> >> >
> >> > There are two nice ways to do this with the clock framework.
> >> > First is explicit re-parenting with voltage scaling done in the
> >> > clock rate-change notifiers:
> >> >
> >> > clk_set_parent(cpu_clk, temp_parent);
> >> > /* implicit voltage scaling to "safe voltage" happens above */
> >> > clk_set_rate(pll, some_rate);
> >> > clk_set_parent(cpu_clk, pll);
> >> > /* implicit voltage scaling to nominal OPP voltage happens above
> >> > */
> >> >
> >
> > I must agree with Mike here. In my opinion the above approach is
> > more compliant with CCF (as I've pointed it out in my other comment
> > - the cpu_clk has more than one parent and we could switch between
> > them when needed).
> 
> The mux which is used to re-parent is part of the larger opaque cpu
> clock type (more like the composite clock). So I am not sure how this
> isn't compliant with ccf.

My point here is to use the clk_set_parent() explicitly instead of
changing the mux with writing values directly to registers.

However, I'm also aware, that we must reparent quickly. so I'm OK
with your approach.

> 
> >
> >> > The above sequence would require a separate exnyos CPUfreq
> >> > driver, due to the added clk_set_parent logic.
> >> >
> >> > The second way to do this is to abstract the clk re-muxing logic
> >> > out into the clk driver, which would allow cpufreq-cpu0 to be
> >> > used for the exynos chips.
> >>
> >> This is the approach this patch series takes (patch 2/7). The clock
> >> re-muxing logic is handled by a clock driver code. The difference
> >> from what you suggested is that the safe voltage (that may be
> >> optionally) required before doing the re-muxing is handled here in
> >> cpufreq-cpu0 driver.
> >>
> >> The safe voltage setup can be done in the notifier as you
> >> suggested.
> >
> > If the clk_set_parent() approach is not suitable, then cannot we
> > consider using the one from highbank-cpufreq.c?
> >
> > Here we have cpufreq-cpu0.c which sets voltage of the cpu_clk.
> > In the highbank-cpufreq.c there are clock notifiers to change the
> > voltage.
> >
> > Cannot Exynos reuse such approach? Why shall we pollute
> > cpufreq-cpu0.c with another solution?
> 
> The highbank-cpufreq.c file was introduced because platforms using
> this driver did not have the usual regulator to control the voltage.
> The first commit of this driver explains this (copied below).
> 
> "Highbank processors depend on the external ECME to perform voltage
> management based on a requested frequency. Communication between the
> A9 cores and the ECME happens over the pl320 IPC channel."
> 
> So those platforms had no choice but to use an alternative approach to
> control the voltage (and reuse cpufreq-cpu0 as much as possible). 
> The
> case with exynos is a different one.

Highbank needs to set voltage via IPC, Exynos needs to adjust voltage
when reparenting.

Both can be recognized as unusual cases. That is why I asked if we
could reuse the same approach for Exynos.

> cpufreq-cpu0 is fully re-usable
> for exynos with the additional support for "safe voltage". If we agree
> that there might be existing or future platforms with single
> clock/voltage rail that require the "safe voltage" feature, then
> adding "safe voltage" support in cpufreq-cpu0 driver seems to be the
> right approach.

I think that Shawn's opinion will be final here.

> 
> >
> >> But, doing that in cpufreq-cpu0 driver will help other platforms
> >> reuse this feature if required. Also, if done here, the regulator
> >> handling is localized in this driver which otherwise would need to
> >> be handled in two places, cpufreq-cpu0 driver and the clock
> >> notifier.
> >
> > I think that there is a logical distinction between setting voltage
> > for cpufreq-cpu0 related clock and increasing voltage of reparented
> > clock.
> >
> > The former fits naturally to cpufreq-cpu0, when the latter seems
> > like some corner case (as Mike pointed out) for Exynos.
> 
> Agreed, it is a corner case. But for this corner case, we are
> performing some additional actions on the same regulator which is used
> in the normal functioning of the driver.
> 
> >
> >>
> >> So I tend to prefer the approach in this patch but I am willing to
> >> consider any suggestions.
> >
> > Thomas, what do you think about highbank-cpufreq.c approach (with
> > using clock notifiers)?
> 
> I have made a related comment on this above.
> 
> >
> > Do you think, that it is feasible to reuse it with Exynos?
> 
> highbank cpufreq driver is intended for a different purpose so I don't
> think it can be reused for exynos. Yes, we can make exynos specific
> hacks into highbank driver but how would that be better over the
> approach in this patch?

I think, that I was misunderstood here. I wanted to ask if we could
reuse the clk notifier approach.

> 
> >
> >> Shawn, it would be helpful if you could let
> >> us know your thoughts on this. I am almost done with testing the
> >> v3 of this series and want to post it so if there are any
> >> objections to the changes in this patch, please let me know.
> >>
> >> Thanks,
> >> Thomas.
> >>
> >> >
> >> > I'm more a fan of explicitly listing the Exact Steps for the cpu
> >> > opp transition in a separate exynos-specific CPUfreq driver, but
> >> > that's probably an unpopular view.
> >> >
> >> > Regards,
> >> > Mike
> >> >
> >> >> ---
> >> >>  .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt   |    7 ++++
> >> >>  drivers/cpufreq/cpufreq-cpu0.c                     |   37
> >> >> +++++++++++++++++-- 2 files changed, 40 insertions(+), 4
> >> >> deletions(-)
> >> >>
> >> >> diff --git
> >> >> a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> >> >> b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> >> >> index f055515..37453ab 100644 ---
> >> >> a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt +++
> >> >> b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt @@
> >> >> -19,6 +19,12 @@ Optional properties:
> >> >>  - cooling-min-level:
> >> >>  - cooling-max-level:
> >> >>       Please refer to
> >> >> Documentation/devicetree/bindings/thermal/thermal.txt. +-
> >> >> safe-opp: Certain platforms require that during a opp
> >> >> transition,
> >> >> +  a system should not go below a particular opp level. For such
> >> >> systems,
> >> >> +  this property specifies the minimum opp to be maintained
> >> >> during the
> >> >> +  opp transitions. The safe-opp value is a tuple with first
> >> >> element
> >> >> +  representing the safe frequency and the second element
> >> >> representing the
> >> >> +  safe voltage.
> >> >>
> >> >>  Examples:
> >> >>
> >> >> @@ -36,6 +42,7 @@ cpus {
> >> >>                         396000  950000
> >> >>                         198000  850000
> >> >>                 >;
> >> >> +               safe-opp = <396000 950000>
> >> >>                 clock-latency = <61036>; /* two CLK32 periods */
> >> >>                 #cooling-cells = <2>;
> >> >>                 cooling-min-level = <0>;
> >> >> diff --git a/drivers/cpufreq/cpufreq-cpu0.c
> >> >> b/drivers/cpufreq/cpufreq-cpu0.c index 0c12ffc..075d3d1 100644
> >> >> --- a/drivers/cpufreq/cpufreq-cpu0.c
> >> >> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> >> >> @@ -27,6 +27,8 @@
> >> >>
> >> >>  static unsigned int transition_latency;
> >> >>  static unsigned int voltage_tolerance; /* in percentage */
> >> >> +static unsigned long safe_frequency;
> >> >> +static unsigned long safe_voltage;
> >> >>
> >> >>  static struct device *cpu_dev;
> >> >>  static struct clk *cpu_clk;
> >> >> @@ -64,17 +66,30 @@ static int cpu0_set_target(struct
> >> >> cpufreq_policy *policy, unsigned int index) volt_old =
> >> >> regulator_get_voltage(cpu_reg); }
> >> >>
> >> >> -       pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
> >> >> +       pr_debug("\n\n%u MHz, %ld mV --> %u MHz, %ld mV\n",
> >> >>                  old_freq / 1000, volt_old ? volt_old / 1000 :
> >> >> -1, new_freq / 1000, volt ? volt / 1000 : -1);
> >> >>
> >> >>         /* scaling up?  scale voltage before frequency */
> >> >> -       if (!IS_ERR(cpu_reg) && new_freq > old_freq) {
> >> >> +       if (!IS_ERR(cpu_reg) && new_freq > old_freq &&
> >> >> +                               new_freq >= safe_frequency) {
> >> >>                 ret = regulator_set_voltage_tol(cpu_reg, volt,
> >> >> tol); if (ret) {
> >> >>                         pr_err("failed to scale voltage up:
> >> >> %d\n", ret); return ret;
> >> >>                 }
> >> >> +       } else if (!IS_ERR(cpu_reg) && old_freq <
> >> >> safe_frequency) {
> >> >> +               /*
> >> >> +                * the scaled up voltage level for the new_freq
> >> >> is lower
> >> >> +                * than the safe voltage level. so set
> >> >> safe_voltage
> >> >> +                * as the intermediate voltage level and revert
> >> >> it
> >> >> +                * back after the frequency has been changed.
> >> >> +                */
> >> >> +               ret = regulator_set_voltage_tol(cpu_reg,
> >> >> safe_voltage, tol);
> >> >> +               if (ret) {
> >> >> +                       pr_err("failed to set safe voltage:
> >> >> %d\n", ret);
> >> >> +                       return ret;
> >> >> +               }
> >> >>         }
> >> >>
> >> >>         ret = clk_set_rate(cpu_clk, freq_exact);
> >> >> @@ -86,7 +101,8 @@ static int cpu0_set_target(struct
> >> >> cpufreq_policy *policy, unsigned int index) }
> >> >>
> >> >>         /* scaling down?  scale voltage after frequency */
> >> >> -       if (!IS_ERR(cpu_reg) && new_freq < old_freq) {
> >> >> +       if (!IS_ERR(cpu_reg) &&
> >> >> +                       (new_freq < old_freq || new_freq <
> >> >> safe_frequency)) { ret = regulator_set_voltage_tol(cpu_reg,
> >> >> volt, tol); if (ret) {
> >> >>                         pr_err("failed to scale voltage down:
> >> >> %d\n", ret); @@ -116,6 +132,8 @@ static struct cpufreq_driver
> >> >> cpu0_cpufreq_driver = {
> >> >>
> >> >>  static int cpu0_cpufreq_probe(struct platform_device *pdev)
> >> >>  {
> >> >> +       const struct property *prop;
> >> >> +       struct dev_pm_opp *opp;
> >> >>         struct device_node *np;
> >> >>         int ret;
> >> >>
> >> >> @@ -165,13 +183,24 @@ static int cpu0_cpufreq_probe(struct
> >> >> platform_device *pdev) goto out_put_node;
> >> >>         }
> >> >>
> >> >> +       prop = of_find_property(np, "safe-opp", NULL);
> >> >> +       if (prop) {
> >> >> +               if (prop->value && (prop->length / sizeof(u32))
> >> >> == 2) {
> >> >> +                       const __be32 *val;
> >> >> +                       val = prop->value;
> >> >> +                       safe_frequency = be32_to_cpup(val++);
> >> >> +                       safe_voltage = be32_to_cpup(val);
> >> >> +               } else {
> >> >> +                       pr_err("invalid safe-opp level
> >> >> specified\n");
> >> >> +               }
> >> >> +       }
> >> >> +
> >> >>         of_property_read_u32(np, "voltage-tolerance",
> >> >> &voltage_tolerance);
> >> >>
> >> >>         if (of_property_read_u32(np, "clock-latency",
> >> >> &transition_latency)) transition_latency = CPUFREQ_ETERNAL;
> >> >>
> >> >>         if (!IS_ERR(cpu_reg)) {
> >> >> -               struct dev_pm_opp *opp;
> >> >>                 unsigned long min_uV, max_uV;
> >> >>                 int i;
> >> >>
> >> >> --
> >> >> 1.6.6.rc2
> >> >>
> >
> >
> >
> > --
> > Best regards,
> >
> > Lukasz Majewski
> >
> > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
> 
> Thanks for your comments Lukasz.
> 
> Regards,
> Thomas.
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply

* Building with gcc 4.6.4
From: Russell King - ARM Linux @ 2014-01-28 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <21223.49366.479579.145000@gargle.gargle.HOWL>

On Tue, Jan 28, 2014 at 03:38:14PM +0100, Mikael Pettersson wrote:
> Russell King - ARM Linux writes:
>  > So, yesterday I built gcc 4.6.4 (mainline) for the autobuilder, and the
>  > result is that every build failed with the same error:
>  > 
>  > scripts/mod/empty.c:1:0: error: FPA is unsupported in the AAPCS
>  > 
>  > This seems to be because linux-elf targets default to fpe3 in mainline
>  > gcc, but specifying -mabi=aapcs-linux switches us into EABI mode where
>  > the compiler errors out with the default FPU.
>  > 
>  > Hence, I believe we need this to ensure that a compatible VFP is
>  > selected.  One can argue that building EABI ARMv4 with VFP is silly,
>  > but it seems that's what the gcc folk have decided (rightly or
>  > wrongly.)
>  > 
>  > Maybe this is a bug in mainline GCC - which begs the question why
>  > (presumably, since no one has picked this up) Linaro's toolchain
>  > has fixes but mainline GCC doesn't.
>  > 
>  > Comments?
> 
> Perhaps because most ARM EABI toolchains default to soft-float,
> and the hardfloat ones usually select v6 or v7 + vfp-d16 or neon
> as their defaults, so the archaic FPA is never the default.

soft-float has nothing to do with it, because the kernel always passes
-msoft-float.

> Or are you using an OABI toolchain to compile an EABI kernel?

... which should make no difference what so ever since the kernel should
be passing the appropriate options.  That's why we pass -mabi=aapcs-linux
to the kernel.

-- 
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

* [PATCH] arm/xen: Initialize event channels earlier
From: Julien Grall @ 2014-01-28 14:54 UTC (permalink / raw)
  To: linux-arm-kernel

Event channels driver needs to be initialized very early. Until now, Xen
initialization was done after all CPUs was bring up.

We can safely move the initialization to an early initcall.

Also use a cpu notifier to:
    - Register the VCPU when the CPU is prepared
    - Enable event channel IRQ when the CPU is running

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 arch/arm/xen/enlighten.c |   84 ++++++++++++++++++++++++++++++----------------
 1 file changed, 55 insertions(+), 29 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 293eeea..39b668e 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -23,6 +23,7 @@
 #include <linux/of_address.h>
 #include <linux/cpuidle.h>
 #include <linux/cpufreq.h>
+#include <linux/cpu.h>
 
 #include <linux/mm.h>
 
@@ -154,12 +155,11 @@ int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
 }
 EXPORT_SYMBOL_GPL(xen_unmap_domain_mfn_range);
 
-static void __init xen_percpu_init(void *unused)
+static void xen_percpu_init(int cpu)
 {
 	struct vcpu_register_vcpu_info info;
 	struct vcpu_info *vcpup;
 	int err;
-	int cpu = get_cpu();
 
 	pr_info("Xen: initializing cpu%d\n", cpu);
 	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
@@ -170,9 +170,11 @@ static void __init xen_percpu_init(void *unused)
 	err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, cpu, &info);
 	BUG_ON(err);
 	per_cpu(xen_vcpu, cpu) = vcpup;
+}
 
+static void xen_interrupt_init(void)
+{
 	enable_percpu_irq(xen_events_irq, 0);
-	put_cpu();
 }
 
 static void xen_restart(enum reboot_mode reboot_mode, const char *cmd)
@@ -193,6 +195,36 @@ static void xen_power_off(void)
 		BUG();
 }
 
+static irqreturn_t xen_arm_callback(int irq, void *arg)
+{
+	xen_hvm_evtchn_do_upcall();
+	return IRQ_HANDLED;
+}
+
+static int xen_cpu_notification(struct notifier_block *self,
+				unsigned long action,
+				void *hcpu)
+{
+	int cpu = (long)hcpu;
+
+	switch (action) {
+	case CPU_UP_PREPARE:
+		xen_percpu_init(cpu);
+		break;
+	case CPU_STARTING:
+		xen_interrupt_init();
+		break;
+	default:
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block xen_cpu_notifier = {
+	.notifier_call = xen_cpu_notification,
+};
+
 /*
  * see Documentation/devicetree/bindings/arm/xen.txt for the
  * documentation of the Xen Device Tree format.
@@ -209,6 +241,7 @@ static int __init xen_guest_init(void)
 	const char *xen_prefix = "xen,xen-";
 	struct resource res;
 	phys_addr_t grant_frames;
+	int cpu;
 
 	node = of_find_compatible_node(NULL, NULL, "xen,xen");
 	if (!node) {
@@ -281,9 +314,27 @@ static int __init xen_guest_init(void)
 	disable_cpuidle();
 	disable_cpufreq();
 
+	xen_init_IRQ();
+
+	if (xen_events_irq < 0)
+		return -ENODEV;
+
+	if (request_percpu_irq(xen_events_irq, xen_arm_callback,
+			       "events", &xen_vcpu)) {
+		pr_err("Error request IRQ %d\n", xen_events_irq);
+		return -EINVAL;
+	}
+
+	cpu = get_cpu();
+	xen_percpu_init(cpu);
+	xen_interrupt_init();
+	put_cpu();
+
+	register_cpu_notifier(&xen_cpu_notifier);
+
 	return 0;
 }
-core_initcall(xen_guest_init);
+early_initcall(xen_guest_init);
 
 static int __init xen_pm_init(void)
 {
@@ -297,31 +348,6 @@ static int __init xen_pm_init(void)
 }
 late_initcall(xen_pm_init);
 
-static irqreturn_t xen_arm_callback(int irq, void *arg)
-{
-	xen_hvm_evtchn_do_upcall();
-	return IRQ_HANDLED;
-}
-
-static int __init xen_init_events(void)
-{
-	if (!xen_domain() || xen_events_irq < 0)
-		return -ENODEV;
-
-	xen_init_IRQ();
-
-	if (request_percpu_irq(xen_events_irq, xen_arm_callback,
-			"events", &xen_vcpu)) {
-		pr_err("Error requesting IRQ %d\n", xen_events_irq);
-		return -EINVAL;
-	}
-
-	on_each_cpu(xen_percpu_init, NULL, 0);
-
-	return 0;
-}
-postcore_initcall(xen_init_events);
-
 /* In the hypervisor.S file. */
 EXPORT_SYMBOL_GPL(HYPERVISOR_event_channel_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_grant_table_op);
-- 
1.7.10.4

^ permalink raw reply related

* [RFC 1/1] Device Tree Schema Source format description
From: Grant Likely @ 2014-01-28 14:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3511775.RFRDLM8Ohp@amdc1227>

Some random thoughts ahead of meeting today...

On Fri, 18 Oct 2013 16:57:36 +0200, Tomasz Figa <t.figa@samsung.com> wrote:
> /*
>  * schema.dtss - Sample Device Tree schema file.
>  *
>  * Copyright (C) 2013 Samsung Electronics Co., Ltd.
>  * Author: Tomasz Figa <t.figa@samsung.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 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.
>  */
> 
> /dtss-v1/;

In general I'm concerned about the creation of a new language, even if
it is based on the DTS syntax. It forces us to define grammer that by
necessity will be simple at first just to get things working, but then
need to be extended to handle complex stuff. I've been researching ASN.1
and similar schema languages to see if we can adopt the structure if not
the syntax when writing schema files.

In the mean time I'm now feeling that we should go ahead with the
C-implementation of schema checking just to make some progress and then
migrate the logic out into separately parsed schema files when we have
something better. The low hanging fruit of course is the core bindings.
Interrupts, gpios, regs, clocks, common busses like spi, i2c & pci.

I think the most important thing to get settled now is determining how
we trigger schema checks. For example, given a random node in the tree,
how does the checker know to apply the interrupts and regs tests?

>  * Special keywords:
>  *
>  * /template/
>  *	Defines schema template that can be used to represent generic bindings
>  *	which may be then included in device bindings by /use/ keyword.
>  *	A template can take a set of required or optional parameters.
>  *	Optional parameters need to have default value specified, which is
>  *	used when the parameter is omited from parameters of /use/ clause.
>  *
>  *	Template declaration uses following syntax:
>  *	/template/ template-name {
>  *		/arg/ argument-name-1; // Required argument
>  *		/arg/ argument-name-2 = <1>; // Optional argument
>  *		// Here follows binding specification described further
>  *		// in this document.
>  *	};

I'm wary of the template approach. I think schemas should be schemas,
and it should alwasy be possible for a schema to derive from any other
schema. For instance, a device binding will pull in the interrupt
consumer and reg core schemas. Also, a special purpose UART device
may use a simple extension of the ns16550 binding.

I guess my point is that templates shouldn't be a special case. All
bindings should be derivable, but what we do need is a mechanism to
constrain bindings. For example, the interrupts binding would describe
one or more interrupts in a list, but a given device may need to
constrain a minimum of 2 interrupts. For an initial C implementation that
could merely be a argument when invoking the binding test.

>  *
>  *	A template argument is dereferenced by its name prefixed with
>  *	a dollar ($) sign. An arbitrary property specified in parsed
>  *	tree can be dereferenced by its absolute or relative path or
>  *	by a lookup up the tree (interrupt-parent like).
>  *
>  * /arg/
>  *	Marks following property declaration as a template argument.
>  *	Can be used only inside a template declaration, to declare template
>  *	arguments, as described above in /template/ keyword description.
>  *
>  * /use/
>  *	Instantiates binding template. Causes the specified template to be
>  *	included as a part of the binding specified by current node.
>  *
>  * 	Syntax used to instatiate templates:
>  *	/use/ template-name {
>  *		// List of template arguments and their values, e.g.
>  *		argument-name-1 = <int-value>;
>  *		argument-name-2 = "string-value";
>  *		// etc.
>  *	};

On the syntax, this feels very verbose. If we were to draw inspiration
from the ASN.1 syntax, we might do somehting like this:

FdtDevice ::= SET {
	interrupts FdtInterrupt (SIZE(1))
	reg FdtReg
}

FdtDevice is a type that makes use of the FdtReg and FdtInterrupt types.

>  *
>  * /incomplete/
>  *	Tells the validator that the schema is not complete, i.e.
>  *	only specified properties and subnodes should be validated,
>  *	without complaining about ones not specified in the schema.
>  *
>  *	Example of use:
>  *	{
>  *		compatible = "simple-bus";
>  *		#address-cells = <1>;
>  *		#size-cells = <1>;
>  *
>  *		*.* { // Zero or more subnodes
>  *			/incomplete/; // of unspecified contents
>  *		};
>  *	};
>  *
>  * /inheritable/
>  *	Marks property as inheritable by subnodes of node in which
>  *	it is defined. An example of such property is interrupt-parent,
>  *	which, if not specified in the same node as interrupts property,
>  *	is assumed to be the same as the closest interrupt-parent property
>  *	found when walking up the tree.
>  *
>  * 	This is purely a helper construct to allow referencing to
>  *	a property from inside of a DTSS, even if it is not specified
>  *	in referred node explicitly.
>  *
>  *	Example of use:
>  *	{
>  *		/inheritable/ interrupt-parent = phandle;
>  *
>  *		.* {
>  *			// interrupt-parent is defined for this node
>  *			// implicitly, even if not specified explicitly
>  *		};
>  *	};

Some bindings are just core, like interrupts. It would probablay be
better to just leave those in C even when we're happy with a schema
language.

>  * Binding description
>  *
>  * A binding starts with a top level node that must be described using at
>  * least one of the key node attributes specified below. The top level node
>  * can contains a set of properties and subnodes that describe the binding.
>  * They can be specified using the DTS Schema syntax, which is basically
>  * the standard DTS syntax modified to convey information about the schema,
>  * not contents.

It seems to me that what you're getting at is that a binding needs to
describe how it is matched (key attributes) and then the set of
constraints. That is a good start, but it feels to me like the matching
isn't very well defined yet. We need some pretty firm rules about how
the checker know when it can apply rules, and those rules will change
depending on where in the tree you are. For example, the cpus schema
only is applicable under the /cpus node, and device nodes with
compatible properties should only match on nodes that are under bus
nodes. (there's another issue of checking for nodes placed where they
shouldn't be, but I'll leave that aside for now).

Also the key properties or matching criteria will not be present for
some bindings because they are only ever directly instantiated. ie. the
interrupts binding doesn't stand on its own, it is only ever called out
to by another binding. (I know, you've addressed some of this above, but
I'm working through a though process).

As we talked about on the call today, if we can hammer out how the
schemas will get selected at test time, then we can start to implement a
hybrid approach where core stuff is implemented in C code and schema
files can instantiate the core bindings as needed.

g.

>  * 
>  * Node description
>  *
>  * A node starts with a node-name, then { and ends with matching } and
>  * a semicolon. It can be described by a set of key attributes, which are:
>  * - compatible property,
>  * - device_type property,
>  * - node name (DEPRECATED for top level nodes of bindings).
>  *
>  * Once a node matches all specified key attributes of a defined binding
>  * it is validated according to description of this binding.
>  *
>  * Property specification
>  *
>  * Each property inside a node is specified just as in a normal DTS file,
>  * except that the value specified determines value type and range of
>  * allowed values, not a particular value itself. In addition, property
>  * name can be prefixed with an optionality quantifier, marking the property
>  * as optional (specified 0 or 1 times).
>  *
>  * Property type can be specified using type specification syntax. It is
>  * a regular expression like syntax, but including special type keywords,
>  * that represent all the defined DTS value types, which is:
>  *  1) string: a string of characters enclosed by quotation marks,
>  *  2) cell: a single, unsigned 32-bit value,
>  *  3) phandle: a valid phandle to any device node,
>  *  4) binary: a sequence of bytes (NOTE: quantifiers work as byte count here).
>  * Property types can be mixed inside the property, so the value can consist
>  * of followed multiple blocks of data of different types.
>  *
>  * In addition, string and cell types can be limited to specified set
>  * of allowed values, discrete or contiguous (cell only), by using
>  * parentheses after type name. Regular expressions are allowed when
>  * specifying allowed string values.
>  *
>  * Inside property specifiers, other properties and template arguments
>  * can be dereferenced by preceding their names with dollar "$" sign.
>  * Element count of a property that is an array can be received by
>  * preceding its name with at "@" sign. The property name following
>  * $ or @ sign is interpreted as a path in device tree, either absolute
>  * if started by a slash "/" or relative otherwise.
>  *
>  * Example properties built from strings:
>  *
>  *	string-property-1 = string; // one string
>  *	string-property-2 = string+; // array of strings
>  *	string-property-3 = string{@clock-names}; // same count as in
>  *						  // clock-names property
>  *	string-property-4 = string("one", "two", "three"); // three
>  *						// allowed values
>  *	string-property-5 = string("[a-zA-Z]+[0-9]"); // allowed values
>  *						// defined using a regexp
>  *
>  * Example properties built from cells:
>  *
>  *	cell-property-1 = cell; // one cell
>  *	cell-property-2 = (cell{$#cell-property-2-cells})+; // cell count
>  *			// specified by #cell-property-name-cells property
>  *	cell-property-3 = cell(0, 1, 2, 3, 4); // integers from 0 to 4
>  *	cell-property-4 = cell(0..4); // same as above, but using a range
>  *
>  * Example proprerties built from phandles:
>  *
>  *	phandle-property-1 = phandle; // one phandle
>  *
>  * Example properties built from binary data:
>  *
>  *	binary-property-1 = binary+; // one or more bytes of binary data
>  *	binary-property-2 = binary{6}; // 6 bytes of binary data
>  *
>  * Example mixed properties:
>  *
>  *	mixed-property-1 = ((phandle),cell{$\2/#mixed-property-1-cells})+;
>  *	// typical specifier - phandle of controller node and a set of cells
>  *	// defined by #*-cells property of controller node. Note the use
>  *	// of backreferences here.
>  *	// TODO: How to represent references to properties in a node
>  *	// pointed by a phandle?
>  *
>  * Quantifiers
>  *
>  * Determines valid iteration count for element after the quantifier
>  * symbol, which can be a property or a child node. <quantifier> can be
>  * any valid quantifier according to POSIX extended regular expression
>  * syntax.
>  *
>  * Examples of use:
>  *
>  * // For properties
>  * property-name-1; // Required property
>  * ?property-name-2; // Optional property
>  *
>  * // For subnodes
>  * node-name-1 {}; // Required node (exactly 1 time)
>  * ?node-name-2 {}; // Optional node (0 or 1 times)
>  * +node-name-3 {}; // Required node (1 or more times)
>  * *node-name-4 {}; // Optional nodes (0 or more times)
>  * {1,3}node-name-5 {}; // From 1 to 3 subnodes must be present
>  *
>  * // For data types
>  * property-name-1 = cell; // One cell
>  * property-name-2 = string+; // One or more strings
>  * property-name-3 = phandle{3,8}; // From three to eight phandles
>  *
>  * Real-life examples
>  *
>  * Following is a set of examples of DTS Schema syntax use, based on existing
>  * device tree bindings. All examples are commented to explain used mechanisms.
>  * Note that all the examples are valid and should compile fine using a DTS
>  * Schema enabled version of the DTC compiler.
>  */
> 
> /*
>  * Utility schema templates.
>  */
> /template/ reg-property {
> 	/*
> 	 * This template takes one argument reg-count, which is optional
> 	 * and if omitted, takes the default value of 1.
> 	 */
> 	/arg/ reg-count = <1>;
> 
> 	/*
> 	 * The example below defines a mandatory "reg" property, consisting
> 	 * of exactly reg-count groups, each consisting of (#address-cells +
> 	 * #size-cells) cells each.
> 	 */
> 	reg = (cell{$../#address-cells},cell{$../#size-cells}){$reg-count};
> };
> 
> /template/ status-property {
> 	/*
> 	 * Template without arguments.
> 	 *
> 	 * Defines a single, optional status property that can take one of
> 	 * enumerated values.
> 	 */
> 	?status = string("okay", "disabled");
> };
> 
> /template/ device {
> 	/* Optional argument, defaulting to 1. */
> 
> 	/* TODO: How to specify variable entry count? */
> 	/arg/ reg-count = <1>;
> 
> 	/* Use a template. */
> 	/use/ reg-property {
> 		/*
> 		 * Specify value of template argument.
> 		 * Note the reference to argument of this template.
> 		 */
> 		reg-count = $reg-count;
> 	};
> 	/* Use another template. This time without arguments specified. */
> 	/use/ status-property;
> };
> 
> /template/ bus {
> 	#address-cells = cell;
> 	#size-cells = cell;
> 
> 	/default/ #address-cells = <2>;
> 	/default/ #size-cells = <1>;
> };
> 
> /*
>  * Generic SPI bindings.
>  */
> /template/ spi {
> 	/* Mandatory argument, without default value. */
> 	/arg/ cs-cells = cell;
> 
> 	/* #address-cells property equal to cs-cells argument. */
> 	#address-cells = <$cs-cells>;
> 	/* Fixed value of #size-cells property. */
> 	#size-cells = <0>;
> 	/* A single-cell property num-cs. */
> 	num-cs = cell;
> 
> 	/* From 0 to $num-cs subnodes with any name. */
> 	{0,$num-cs}.* {
> 		/* This node must have a reg property, with one entry. */
> 		/use/ reg-property {
> 			reg-count = <1>;
> 		};
> 		/* This binding does not fully define node contents. */
> 		/incomplete/;
> 	};
> };
> 
> /*
>  * Generic interrupt bindings.
>  */
> /template/ interrupts {
> 	/*
> 	 * Optional argument, specifying number of entries in interrupts
> 	 * property. Defaults to 1.
> 	 * TODO: How to specify optional interrupts or interrupt lists
> 	 * that vary with compatible string?
> 	 */
> 	/arg/ interrupt-count = <1>;
> 
> 	/* Optional phandle to interrupt controller. */
> 	/inheritable/ ?interrupt-parent = phandle;
> 	/*
> 	 * List of interrupts.
> 	 * TODO: variable interrupt count?
> 	 */
> 	interrupts = (cell{$($interrupt-parent)/#interrupt-cells}){$interrupt-count};
> };
> 
> /*
>  * Generic clock bindings.
>  */
> /template/ clocks {
> 	/*
> 	 * Required argument specifying number of clocks.
> 	 * TODO: Optional clocks?
> 	 */
> 	/arg/ clock-count = cell;
> 
> 	/*
> 	 * List of exactly $count clock specifiers.
> 	 * TODO: How to dereference phandles of specifiers properly.
> 	 */
> 	clocks = ((phandle),cell{$\2/#clock-cells}){$count};
> };
> 
> /*
>  * Generic pinctrl bindings.
>  */
> /template/ pinctrl {
> 	/*
> 	 * List of pinctrl names.
> 	 * TODO: Optional pinctrl states?
> 	 */
> 	/arg/ names = string+;
> 
> 	/* Pinctrl names, as specified in $names. */
> 	pinctrl-names = $names;
> 	/*
> 	 * Pinctrl groups for every entry of pinctrl-names property.
> 	 * TODO: Find a correct way to define a series of properties.
> 	 */
> 	pinctrl-[0- at names] = phandle+;
> };
> 
> /template/ pinctrl-default {
> 	/* Fixed value of required pinctrl-names property. */
> 	pinctrl-names = "default";
> 	/*
> 	 * List of pinctrl groups for default pinctrl state.
> 	 * NOTE that an empty list of groups can be specified too.
> 	 */
> 	pinctrl-0 = phandle*;
> };
> 
> /*
>  * Generic video interface (V4L2/media) bindings.
>  */
> /template/ video-port {
> 	port {
> 		/* Bus with 1-cell addresses and no sizes. */
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		/* One or more endpoint nodes. */
> 		+endpoint {
> 			/* Single entry reg property. */
> 			/use/ reg-property;
> 			/*
> 			 * Required phandle to remote endpoint.
> 			 * TODO: Do we need to check things like bidirectional
> 			 * connectivity? I.e. whether the node pointed by
> 			 * $remote-endpoint points to this node?
> 			 */
> 			remote-endpoint = phandle;
> 			/* Optional property indicating slave mode operation. */
> 			?slave-mode;
> 			/* Optional bus width */
> 			?bus-width = cell;
> 			/* ... */
> 		};
> 	};
> };
> 
> /template/ video-ports {
> 	/* Bus with 1-cell addresses and no sizes. */
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 
> 	/* Include video port template. */
> 	/use/ video-port {
> 		/* Override quantifier of port subnodes. */
> 		+port {
> 			/* Single entry reg property. */
> 			/use/ reg-property;
> 		};
> 	};
> };
> 
> /*
>  * Skeleton schema for all DTSes according to ePAPR 1.1.
>  *
>  * NOTE explicit node location specified.
>  */
> &{/} {
> 	/* Human readable board model. */
> 	model = string;
> 	/* Set of board compatible values. */
> 	compatible = string+;
> 	/* Optional version of ePAPR spec this device tree complies to. */
> 	?epapr-version = string;
> 	/* Top level bus */
> 	/use/ bus;
> 
> 	/* Required chosen node. */
> 	chosen {
> 		/* Optional bootargs string. */
> 		?bootargs = string;
> 		/* Optional stdout-path string. */
> 		?stdout-path = string;
> 		/* Optional stdin-path string. */
> 		?stdin-path = string;
> 	};
> 
> 	/* Required aliases node. */
> 	aliases {
> 		/*
> 		 * Optional list of aliases with arbitrary names, pointing
> 		 * to device nodes either by path...
> 		 */
> 		?[a-z0-9_]+ = string; /* TODO: Should we check path validity? */
> 		/* ...or phandle. */
> 		?[a-z0-9_]+ = phandle;
> 	};
> 
> 	/* Required memory node. */
> 	memory {
> 		/* Required single entry reg property. */
> 		/use/ reg-property {
> 			reg-count = /* TODO: how to specify a range here */;
> 		};
> 
> 		/* Required device_type set to "memory". */
> 		device_type = "memory";
> 		/* Optional property containing multiple reg-like entries. */
> 		?initial-mapped-area = (cell{../#address-cells},cell{../#size-cells})+;
> 	};
> 
> 	/* Required cpus node. */
> 	cpus {
> 		/* A bus with 1-cell address and 0-cell size specifiers. */
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		/* A set of CPU nodes. At least one is required. */
> 		+cpu {
> 			/* 1-entry reg property, for CPU MPIDR. */
> 			/use/ reg-property;
> 			/* This node can have status property. */
> 			/use/ status-property;
> 			/* The device_type property must be set to "cpu". */
> 			device_type = "cpu";
> 			/*
> 			 * Optional clock-frequency property, specified
> 			 * using 1 or 2 cells, depending on the platform.
> 			 */
> 			?clock-frequency = cell{1,2};
> 
> 			/*
> 			 * Additional platform-specific properties might be
> 			 * present here, which are out of scope of this
> 			 * binding.
> 			 */
> 			/incomplete/;
> 		};
> 	};
> 
> 	/* Any optional device nodes. */
> 	*.* {
> 		/* Out of scope of this binding. */
> 		/incomplete/;
> 	};
> };
> 
> /*
>  * Davinci SPI controller device bindings
>  */
> {
> 	/* Binding is defined for any of following compatible values. */
> 	compatible = string("ti,dm64410-spi", "ti,da830-spi");
> 
> 	/* Use generic device bindings. */
> 	/use/ device;
> 	/* Use generic SPI bindings. */
> 	/use/ spi {
> 		/* Chip select is identified using one cell. */
> 		cs-cells = <1>;
> 	};
> 	/* Use generic interrupt bindings. */
> 	/use/ interrupts {
> 		/* This device has one interrupt signal. */
> 		interrupt-count = <1>;
> 	};
> 	/* Use generic clock bindings. */
> 	/use/ clocks {
> 		/*
> 		 * This device consumes one clock, without the need to
> 		 * specify its name.
> 		 */
> 		count = <1>;
> 	};
> 
> 	/*
> 	 * Device-specific property that defines which IP interrupt signal
> 	 * is tied to SoC's interrupt controller.
> 	 *
> 	 * TODO: Is this correct?
> 	 */
> 	ti,davinci-spi-intr-line = cell(<0>, <1>);
> };
> 
> /*
>  * Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC)
>  */
> {
> 	/* Binding is defined for following compatible value. */
> 	compatible = "samsung,fimc";
> 
> 	/* Use generic clock bindings. */
> 	/use/ clocks {
> 		clock-count = <4>;
> 	};
> 	/* Clocks with following names must be specified. */
> 	clock-names = "sclk_cam0", "sclk_cam1", "pxl_async0", "pxl_async1";
> 	/* Use generic pinctrl bindings. */
> 	/use/ pinctrl {
> 		/*
> 		 * At least "default" state must be specified. Remaining
> 		 * three states are optional.
> 		 * TODO: Revise handling of optional states.
> 		 */
> 		names = "default", ("idle", "active-a", "active-b")?;
> 	};
> 	/* This node can have a status property. */
> 	/use/ status-property;
> 	/* This node represents a bus with 1-cell addresses and sizes. */
> 	#address-cells = <1>;
> 	#size-cells = <1>;
> 
> 	/* Optional node representing parallel camera ports. */
> 	?parallel-ports {
> 		/*
> 		 * This node represents a bus with 1-cell addresses and
> 		 * no sizes.
> 		 */
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		/* Use generic video port bindings. */
> 		/use/ video-port;
> 	};
> 
> 	/* At least one fimc node is required */
> 	+fimc {
> 		/* Compatible should be set to one of following values. */
> 		compatible = string("samsung,s5pv210-fimc",
> 				  "samsung,exynos4210-fimc",
> 				  "samsung,exynos4212-fimc");
> 		/* This is a device. */
> 		/use/ device;
> 		/* This device has a single interrupt signal. */
> 		/use/ interrupts;
> 		/* Clocks are used. */
> 		/use/ clocks {
> 			clock-count = <2>;
> 		};
> 		/* With following clock names. */
> 		clock-names = "fimc", "sclk_fimc";
> 
> 		/*
> 		 * Here follows an example set of properties described
> 		 * as in existing binding documentation files.
> 		 * 
> 		 * NOTE: Each property is followed by its description.
> 		 */
> 
> 		/* Required properties: */
> 		samsung,pix-limits = cell{4};
> 		/*
> 		 * an array of maximum supported image sizes in pixels, for
> 		 * details refer to Table 2-1 in the S5PV210 SoC User Manual; The meaning of
> 		 * each cell is as follows:
> 		 * 0 - scaler input horizontal size,
> 		 * 1 - input horizontal size for the scaler bypassed,
> 		 * 2 - REAL_WIDTH without input rotation,
> 		 * 3 - REAL_HEIGHT with input rotation.
> 		 */
> 		samsung,sysreg = phandle;
> 		/* A phandle to the SYSREG node. */
> 
> 		/* Optional properties: */
> 		?clock-frequency = cell;
> 		/* maximum FIMC local clock (LCLK) frequency */
> 		?samsung,min-pix-sizes = cell{2};
> 		/*
> 		 * An array specifying minimum image size in pixels at
> 		 * the FIMC input and output DMA, in the first and second cell
> 		 * respectively. Default value when this property is not
> 		 * present is <16 16>
> 		 */
> 		?samsung,min-pix-alignment = cell{2};
> 		/*
> 		 * minimum supported image height alignment (first cell)
> 		 * and the horizontal image offset (second cell). The values
> 		 * are in pixels and default to <2 1> when this property is
> 		 * not present
> 		 */
> 		?samsung,mainscaler-ext;
> 		/*
> 		 * A boolean property indicating whether the FIMC IP
> 		 * supports extended image size and has CIEXTEN register.
> 		 */
> 		?samsung,rotators = cell;
> 		/*
> 		 * A bitmask specifying whether this IP has the input and
> 		 * the output rotator. Bits 4 and 0 correspond to input and
> 		 * output rotator respectively. If a rotator is present its
> 		 * corresponding bit should be set. Default value when this
> 		 * property is not specified is 0x11.
> 		 */
> 		?samsung,cam-if;
> 		/*
> 		 * A bolean property indicating whether the IP block includes
> 		 * the camera input interface.
> 		 */
> 		?samsung,isp-wb;
> 		/*
> 		 * This property must be present if the IP block has the ISP
> 		 * writeback input.
> 		 */
> 		?samsung,lcd-wb;
> 		/*
> 		 * This property must be present if the IP block has the LCD
> 		 * writeback input.
> 		 */
> 	};
> 
> 	/*
> 	 * One or more optional csis node.
> 	 * Contents unspecified by this binding.
> 	 */
> 	*csis {
> 		/incomplete/;
> 	};
> 
> 	/*
> 	 * One or more optional fimc-lite node.
> 	 * Contents unspecified by this binding.
> 	 */
> 	*fimc-lite {
> 		/incomplete/;
> 	};
> };
> 
> /*
>  * PCI device binding template (used by PCI bus binding)
>  */
> /template/ pci-device {
> 	/* Compatible string matching any of listed regular expressions. */
> 	compatible = string("pciclass,([0-9a-f]{4})([0-9a-f]{2})?",
> 			"pci([1-9a-f][0-9a-f]{0,3}|0),([1-9a-f][0-9a-f]{0,3}|0)\.([1-9a-f][0-9a-f]{0,3})\.([1-9a-f][0-9a-f]{0,3}|0)\.([1-9a-f][0-9a-f]?|0)",
> 			"pci([1-9a-f][0-9a-f]{0,3}|0),([1-9a-f][0-9a-f]{0,3}|0)\.([1-9a-f][0-9a-f]{0,3})\.([1-9a-f][0-9a-f]{0,3}|0)",
> 			"pci([1-9a-f][0-9a-f]{0,3}),([1-9a-f][0-9a-f]{0,3}|0)",
> 			"pci([1-9a-f][0-9a-f]{0,3}|0),([1-9a-f][0-9a-f]{0,3}|0)\.([1-9a-f][0-9a-f]?|0)",
> 			"pci([1-9a-f][0-9a-f]{0,3}|0),([1-9a-f][0-9a-f]{0,3}|0)",
> 			"pciclass,([0-9a-f]{4})([0-9a-f]{2})?");
> 	/* Device name string. */
> 	name = string;
> 	/*
> 	 * A set of 5-cell reg entries defined accordign to PCI binding
> 	 * specification.
> 	 */
> 	reg = (cell{5})+;
> 	/* Optional interrupt specifier. */
> 	/use/ ?interrupts = {
> 		interrupt-count = <1>;
> 	};
> 
> 	/* Optional alternate reg property. */
> 	?alternate-reg = (cell{5})+;
> 	/* Optional single cell property. */
> 	?fcode-rom-offset = cell;
> 	/* Optional list of up to 6 Assigned addresses. */
> 	?assigned-addresses = (cell{5}){0,6};
> 	/* Optional list of power consumption values - from 1 to 10 cells. */
> 	?power-consumption = cell{1,10};
> 
> 	/*
> 	 * Simple single cell properties defined in PCI binding
> 	 * specification.
> 	 */
> 	vendor-id = cell;
> 	device-id = cell;
> 	revision-id = cell;
> 	class-code = cell;
> 	min-grant = cell;
> 	max-latency = cell;
> 	devsel-speed = cell;
> 	?cache-line-size = cell;
> 	?fast-back-to-back;
> 	?subsystem-id = cell;
> 	?subsystem-vendor-id = cell;
> 
> 	/* Boolean properties. */
> 	?66mhz-capable;
> 	?udf-supported;
> 
> 	/* Extra per-device attributes are allowed. */
> 	/incomplete/;
> };
> 
> /*
>  * PCI bus binding template
>  */
> /template/ pci-bus {
> 	/* PCI bus node must have device_type set to "pci". */
> 	device_type = "pci";
> 	/* Standard bus attributes. */
> 	#address-cells = <3>;
> 	#size-cells = <2>;
> 	ranges;
> 
> 	/* A set of optional properties. */
> 	?bus-range = cell{2};
> 	?clock-frequency = cell;
> 	?slot-names = cell, string+;
> 	?bus-master-capable = cell;
> 
> 	/* PCI devices */
> 	* {
> 		/use/ pci-device;
> 	};
> };
> 
> /*
>  * NVIDIA Tegra PCIe controller
>  */
> {
> 	/* Binding is identified by following compatible list. */
> 	compatible = string("nvidia,tegra20-pcie", "nvidia,tegra30-pcie");
> 	/* It is a PCI bus. */
> 	/use/ pci-bus;
> 	/* Needs 3 reg entries. */
> 	/use/ reg-property {
> 		reg-count = <3>;
> 	};
> 	/* Names of reg entries must be specified as follows. */
> 	reg-names = "pads", "afi", "cs";
> 	/* Neds 2 interrupts. */
> 	/use/ interrupts {
> 		interrupt-count = <2>;
> 	};
> 	/* With names as specified below. */
> 	interrupt-names = "intr", "msi";
> 	/* Needs 5 clocks. */
> 	/use/ clocks {
> 		clock-count = <5>;
> 	};
> 	/* With following input names. */
> 	clock-names = "pex", "afi", "pcie_xclk", "pll_e", "cml";
> 	/* Needs pex-clk regulator. */
> 	/use/ regulator {
> 		supply-name = "pex-clk";
> 	};
> 	/* Needs vdd regulator. */
> 	/use/ regulator {
> 		supply-name = "vdd";
> 	};
> 	/* Needs avdd regulator. */
> 	/use/ regulator {
> 		supply-name = "avdd";
> 	};
> 
> 	/* Root port nodes. */
> 	*pci {
> 		/use/ pci-bus;
> 		nvidia,num-lanes = cell;
> 	};
> };
> 

^ permalink raw reply

* Building with gcc 4.6.4
From: Mikael Pettersson @ 2014-01-28 14:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140128120150.GG15937@n2100.arm.linux.org.uk>

Russell King - ARM Linux writes:
 > So, yesterday I built gcc 4.6.4 (mainline) for the autobuilder, and the
 > result is that every build failed with the same error:
 > 
 > scripts/mod/empty.c:1:0: error: FPA is unsupported in the AAPCS
 > 
 > This seems to be because linux-elf targets default to fpe3 in mainline
 > gcc, but specifying -mabi=aapcs-linux switches us into EABI mode where
 > the compiler errors out with the default FPU.
 > 
 > Hence, I believe we need this to ensure that a compatible VFP is
 > selected.  One can argue that building EABI ARMv4 with VFP is silly,
 > but it seems that's what the gcc folk have decided (rightly or
 > wrongly.)
 > 
 > Maybe this is a bug in mainline GCC - which begs the question why
 > (presumably, since no one has picked this up) Linaro's toolchain
 > has fixes but mainline GCC doesn't.
 > 
 > Comments?

Perhaps because most ARM EABI toolchains default to soft-float,
and the hardfloat ones usually select v6 or v7 + vfp-d16 or neon
as their defaults, so the archaic FPA is never the default.

Or are you using an OABI toolchain to compile an EABI kernel?

/Mikael

^ permalink raw reply

* [PATCH 01/11] resolve PXA<->8250 serial device address conflict
From: Sergei Ianovich @ 2014-01-28 14:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140128141437.GB8713@xo-6d-61-c0.localdomain>

On Tue, 2014-01-28 at 15:14 +0100, Pavel Machek wrote:
> Yes, please. I was hitting the same issue with Sharp Zaurus and bluetooth
> CF card... Bluetooth card had 8250 inside...

A better implementation is posted as v3 of the patch.

http://linux-kernel.2935.n7.nabble.com/PATCH-00-11-ARM-support-for-ICP-DAS-LP-8x4x-tp761919p773485.html

It would be great if you could test that patch add post back the
results.

^ permalink raw reply

* [linux-pm] ARM hibernation / suspend-to-disk
From: Pavel Machek @ 2014-01-28 14:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <51A6399D.4090606@ti.com>

Hi!

> > what's the status of suspend-to-disk on ARM? The most recent discussion I
> > found is:
> > http://lists.linuxfoundation.org/pipermail/linux-pm/2012-November/034997.html
> > 
> > with no replies at all. Is anyone still working on that? Anyone got it running?
> > 
> > I tried the patch above (on top of LTS 3.4) and got stuck forever, the last
> > thing on the console was:
> > 
> > root at omap5:/sys/power# echo disk > state
> > [ 2015.641540] PM: Syncing filesystems ... done.
> > [ 2015.666870] PM: Preallocating image memory... done (allocated 16957 pages)
> > [ 2016.062011] PM: Allocated 67828 kbytes in 0.38 seconds (178.49 MB/s)
> > 
> > Any hint how to debug that?
> 
> Because of the nature of omap5 PM, you'd need omap5 specific support in
> order for this to work. Specifically, there are a number of assumptions
> about certain power domains never losing state. You can take a look at
> the work I did to get this working on am335x here:
> 
> https://github.com/russdill/linux/commits/arm-hibernation-am33xx
> 
> For ARM hibernation support to get merged, there needs to be at least
> one platform that supports it. the am335x code I have is not ready as it
> still relies on PM patchsets that have not yet themselves been merged.

Given that we have chicken-and-egg problem here, I believe it makes sense
to merge generic ARM hibernation support now.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* [PATCH 01/11] resolve PXA<->8250 serial device address conflict
From: Pavel Machek @ 2014-01-28 14:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1385879185-22455-2-git-send-email-ynvich@gmail.com>

Hi!

> PXA serial ports have "standard" UART names (ttyS[0-3]), major
> device number (4) and first minor device number (64) by default.
> 
> If the system has extra 8250 serial port hardware in addition
> to onboard PXA serial ports, default settings produce a device
> allocation conflict.
> 
> The patch provides a configuration option which can move onboard
> ports out of the way of 8250_core by assigning a different (204)
> major number and corresponding device names (ttySA[0-3]).

Yes, please. I was hitting the same issue with Sharp Zaurus and bluetooth
CF card... Bluetooth card had 8250 inside...


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* [Q] block / zynq: DMA bouncing
From: Michal Simek @ 2014-01-28 13:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140128134827.GK15937@n2100.arm.linux.org.uk>

On 01/28/2014 02:48 PM, Russell King - ARM Linux wrote:
> On Tue, Jan 28, 2014 at 02:28:28PM +0100, Guennadi Liakhovetski wrote:
>> +static void __init zynq_memory_init(void)
>> +{
>> +	/*
>> +	 * Reserve the 0-0x4000 addresses (before page tables and kernel)
>> +	 * which can't be used for DMA
>> +	 */
>> +	if (!__pa(PAGE_OFFSET))
>> +		memblock_reserve(0, 0x4000);
> 
> Or maybe this:
> 
> 	memblock_reserve(__pa(PAGE_OFFSET), __pa(swapper_pg_dir));
> 
> since that's actually what you mean here.

yep with that if too.

I will prepare that patch and will send it out.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140128/a500c3b6/attachment.sig>

^ permalink raw reply

* [Q] block / zynq: DMA bouncing
From: Russell King - ARM Linux @ 2014-01-28 13:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <Pine.LNX.4.64.1401281420580.2714@axis700.grange>

On Tue, Jan 28, 2014 at 02:28:28PM +0100, Guennadi Liakhovetski wrote:
> +static void __init zynq_memory_init(void)
> +{
> +	/*
> +	 * Reserve the 0-0x4000 addresses (before page tables and kernel)
> +	 * which can't be used for DMA
> +	 */
> +	if (!__pa(PAGE_OFFSET))
> +		memblock_reserve(0, 0x4000);

Or maybe this:

	memblock_reserve(__pa(PAGE_OFFSET), __pa(swapper_pg_dir));

since that's actually what you mean here.

-- 
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

* [Q] block / zynq: DMA bouncing
From: Guennadi Liakhovetski @ 2014-01-28 13:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <Pine.LNX.4.64.1401281420580.2714@axis700.grange>

On Tue, 28 Jan 2014, Guennadi Liakhovetski wrote:

> Hi Michal,
> 
> (trimmed CC a bit)
> 
> On Mon, 27 Jan 2014, Michal Simek wrote:
> 
> > On 01/27/2014 06:52 PM, Russell King - ARM Linux wrote:
> > > On Mon, Jan 27, 2014 at 06:45:50PM +0100, Michal Simek wrote:
> > >> Why 0x4000? IRC Linux for ARM is using space for any purpose.
> > >> Russell knows this much better than I.
> > > 
> > > Probably because as the kernel is loaded at 0x8000, it will place the
> > > swapper page table at 0x4000, thus covering from 0x4000 upwards.
> > 
> > Ah yeah swapper.
> > 
> > > 
> > > Thus, the majority of your un-DMA-able memory will be kernel text or
> > > swapper page tables.
> > 
> > Yes, exactly.
> > 0x0 - 0x4000 - reserving not to be used by DMA
> > 0x4000 - 0x8000 swapper page table
> > 0x8000 - 0x80000 kernel text + up
> 
> Maybe you could submit something like the attached patch upstream. I'm not 
> sure shom you'd like to put as its original author. I put John Linn there 
> as he originally authored commit 5b9f3f2ac8a3e4edae9de7b855f25f757884d84c 

Sorry, I actually meant commit 83e198c01c381a1d90ba07e241a517d1dabf7c84

Guennadi

> from Xilinx tree with the same description, as the attached patch. 
> Unfortunately, that commit didn't have an Sob.

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* [Q] block / zynq: DMA bouncing
From: Guennadi Liakhovetski @ 2014-01-28 13:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52E69E1E.3020202@monstr.eu>

Hi Michal,

(trimmed CC a bit)

On Mon, 27 Jan 2014, Michal Simek wrote:

> On 01/27/2014 06:52 PM, Russell King - ARM Linux wrote:
> > On Mon, Jan 27, 2014 at 06:45:50PM +0100, Michal Simek wrote:
> >> Why 0x4000? IRC Linux for ARM is using space for any purpose.
> >> Russell knows this much better than I.
> > 
> > Probably because as the kernel is loaded at 0x8000, it will place the
> > swapper page table at 0x4000, thus covering from 0x4000 upwards.
> 
> Ah yeah swapper.
> 
> > 
> > Thus, the majority of your un-DMA-able memory will be kernel text or
> > swapper page tables.
> 
> Yes, exactly.
> 0x0 - 0x4000 - reserving not to be used by DMA
> 0x4000 - 0x8000 swapper page table
> 0x8000 - 0x80000 kernel text + up

Maybe you could submit something like the attached patch upstream. I'm not 
sure shom you'd like to put as its original author. I put John Linn there 
as he originally authored commit 5b9f3f2ac8a3e4edae9de7b855f25f757884d84c 
from Xilinx tree with the same description, as the attached patch. 
Unfortunately, that commit didn't have an Sob.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Xilinx-ARM-BSP-prevent-DMA-into-lower-memory.patch
Type: text/x-diff
Size: 1867 bytes
Desc: 
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140128/50003bc6/attachment.bin>

^ permalink raw reply

* [Patch v3 2/2] dmaengine: qcom_bam_dma: Add device tree binding
From: Russell King - ARM Linux @ 2014-01-28 13:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4916428.f0GtxdkWKj@wuerfel>

On Tue, Jan 28, 2014 at 01:05:10PM +0100, Arnd Bergmann wrote:
> Ok, thanks for clearing up my mistake. However, the argument remains:
> the direction doesn't need to be in the DT DMA descriptor since it
> gets set by software anyway.

Yes - for full-duplex, it's implied, since you have one DMA request
(and therefore virtual channel) for memory-to-device and another for
device-to-memory.

-- 
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

* [PATCH v2 1/7] cpufreq: cpufreq-cpu0: allow optional safe voltage during frequency transitions
From: Thomas Abraham @ 2014-01-28 12:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140128114932.GE20583@S2101-09.ap.freescale.net>

On Tue, Jan 28, 2014 at 5:19 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Tue, Jan 28, 2014 at 11:00:29AM +0530, Thomas Abraham wrote:
>> Hi Mike,
>>
>> On Tue, Jan 28, 2014 at 1:55 AM, Mike Turquette <mturquette@linaro.org> wrote:
>> > Quoting Thomas Abraham (2014-01-18 04:10:51)
>> >> From: Thomas Abraham <thomas.ab@samsung.com>
>> >>
>> >> On some platforms such as the Samsung Exynos, changing the frequency
>> >> of the CPU clock requires changing the frequency of the PLL that is
>> >> supplying the CPU clock. To change the frequency of the PLL, the CPU
>> >> clock is temporarily reparented to another parent clock.
>> >>
>> >> The clock frequency of this temporary parent clock could be much higher
>> >> than the clock frequency of the PLL at the time of reparenting. Due
>> >> to the temporary increase in the CPU clock speed, the CPU (and any other
>> >> components in the CPU clock domain such as dividers, mux, etc.) have to
>> >> to be operated at a higher voltage level, called the safe voltage level.
>> >> This patch adds optional support to temporarily switch to a safe voltage
>> >> level during CPU frequency transitions.
>> >>
>> >> Cc: Shawn Guo <shawn.guo@linaro.org>
>> >> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>> >
>> > I'm not a fan of this change. This corner case should be abstracted away
>> > somehow. I had talked to Chander Kayshap previously about handling
>> > voltage changes in clock notifier callbacks, which then renders any
>> > voltage change as a trivial part of the clock rate transition. That
>> > means that this "safe voltage" thing could be handled automagically
>> > without any additional code in the CPUfreq driver.
>> >
>> > There are two nice ways to do this with the clock framework. First is
>> > explicit re-parenting with voltage scaling done in the clock rate-change
>> > notifiers:
>> >
>> > clk_set_parent(cpu_clk, temp_parent);
>> > /* implicit voltage scaling to "safe voltage" happens above */
>> > clk_set_rate(pll, some_rate);
>> > clk_set_parent(cpu_clk, pll);
>> > /* implicit voltage scaling to nominal OPP voltage happens above */
>> >
>> > The above sequence would require a separate exnyos CPUfreq driver, due
>> > to the added clk_set_parent logic.
>> >
>> > The second way to do this is to abstract the clk re-muxing logic out
>> > into the clk driver, which would allow cpufreq-cpu0 to be used for the
>> > exynos chips.
>>
>> This is the approach this patch series takes (patch 2/7). The clock
>> re-muxing logic is handled by a clock driver code. The difference from
>> what you suggested is that the safe voltage (that may be optionally)
>> required before doing the re-muxing is handled here in cpufreq-cpu0
>> driver.
>>
>> The safe voltage setup can be done in the notifier as you suggested.
>> But, doing that in cpufreq-cpu0 driver will help other platforms reuse
>> this feature if required. Also, if done here, the regulator handling
>> is localized in this driver which otherwise would need to be handled
>> in two places, cpufreq-cpu0 driver and the clock notifier.
>>
>> So I tend to prefer the approach in this patch but I am willing to
>> consider any suggestions. Shawn, it would be helpful if you could let
>> us know your thoughts on this. I am almost done with testing the v3 of
>> this series and want to post it so if there are any objections to the
>> changes in this patch, please let me know.
>
> To me, it's the best that we reuse cpufreq-cpu0 for exynos without any
> changes on cpufreq-cpu0 driver ;)

Okay, so that leaves us with the only option of handling "safe
voltage" using clock notifier callbacks as suggested by Mike. So there
are two options - a samsung specific cpufreq driver handling the clock
notifiers (and reusing cpufreq-cpu0 driver) or the samsung clock
driver handles the clock notifiers (and reusing cpufreq-cpu0 driver).

With the second option, the clock driver will have to handle the
regulator lookup from the cpu node, deferring regulator lookup until
the cpu and regulator devices are registered and using regulator api
inside clock driver. This seems like too much code to just manage the
"safe voltage".

So how about the first option of samsung specific cpufreq driver. If
there are any other alternatives, please let me know.

Thanks,
Thomas.

>
> Shawn
>

^ permalink raw reply

* [PATCH 2/2] ARM: dts: imx6sl-evk: Add audio support
From: Shawn Guo @ 2014-01-28 12:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAOMZO5BWfrjxJr=f=0hHwQNuJcQDeLSJi=u6Fci44EctKMSCHQ@mail.gmail.com>

On Tue, Jan 28, 2014 at 10:14:55AM -0200, Fabio Estevam wrote:
> On Tue, Jan 28, 2014 at 10:12 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> 
> >> diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
> >> index 95bb37b..04cf457 100644
> >> --- a/arch/arm/boot/dts/imx6sl.dtsi
> >> +++ b/arch/arm/boot/dts/imx6sl.dtsi
> >> @@ -236,6 +236,7 @@
> >>                                              <&sdma 38 1 0>;
> >>                                       dma-names = "rx", "tx";
> >>                                       fsl,fifo-depth = <15>;
> >> +                                     fsl,ssi-dma-events = <38 37>;
> >
> > Do you still need this custom property?
> 
> I can get rid of it.
> 
> Should I remove from the other SoCs as well?

We have the requirement that a newer kernel must work with all the older
DTBs.  But I haven't heard too much about the opposite - a newer DTB
needs to work with all the older kernel.  If that's case, we can remove
it from other SoCs as well, since the latest kernel already moves to
generic DMA bindings.

Shawn

^ permalink raw reply

* [PATCH 18/18] mfd: max14577: Add device tree bindings document
From: Krzysztof Kozlowski @ 2014-01-28 12:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390911522-28209-1-git-send-email-k.kozlowski@samsung.com>

Add document describing device tree bindings for MAX14577 MFD driver
(for both MAX14577 and MAX77836 chipsets).

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Tomasz Figa <t.figa@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: devicetree at vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 Documentation/devicetree/bindings/mfd/max14577.txt |  104 ++++++++++++++++++++
 1 file changed, 104 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/max14577.txt

diff --git a/Documentation/devicetree/bindings/mfd/max14577.txt b/Documentation/devicetree/bindings/mfd/max14577.txt
new file mode 100644
index 000000000000..cb758f84be00
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max14577.txt
@@ -0,0 +1,104 @@
+Maxim MAX14577/77836 Multi-Function Device
+
+MAX14577 is a Multi-Function Device with Micro-USB Interface Circuit, Li+
+Battery Charger and SFOUT LDO output for powering USB devices. It is
+interfaced to host controller using I2C.
+
+MAX77836 additionally contains PMIC (with two LDO regulators) and Fuel Gauge.
+
+
+Required properties:
+- compatible : Must be "maxim,max14577" or "maxim,max77836".
+- reg : I2C slave address for the max14577 chip (0x25 for max14577/max77836)
+- interrupts : IRQ line for the chip.
+- interrupt-parent :  The parent interrupt controller.
+
+
+Optional nodes:
+- max14577-muic/max77836-muic :
+	Node used only by extcon consumers.
+	Required properties:
+		- compatible : "maxim,max14577-muic" or "maxim,max77836-muic"
+
+- regulators :
+	Required properties:
+		- compatible : "maxim,max14577-regulator"
+			or "maxim,max77836-regulator"
+
+	May contain a sub-node per regulator from the list below. Each
+	sub-node should contain the constraints and initialization information
+	for that regulator. See regulator.txt for a description of standard
+	properties for these sub-nodes.
+
+	List of valid regulator names:
+	- for max14577: CHARGER, SAFEOUT.
+	- for max77836: CHARGER, SAFEOUT, LDO1, LDO2.
+
+	The SAFEOUT is a fixed voltage regulator so there is no need to specify
+	voltages for it.
+
+
+Example:
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+max14577 at 25 {
+	compatible = "maxim,max14577";
+	reg = <0x25>;
+	interrupt-parent = <&gpx1>;
+	interrupts = <5 IRQ_TYPE_NONE>;
+
+	muic: max14577-muic {
+		compatible = "maxim,max14577-muic";
+	};
+
+	regulators {
+		compatible = "maxim,max14577-regulator";
+
+		SAFEOUT {
+			regulator-name = "SAFEOUT";
+		};
+		CHARGER {
+			regulator-name = "CHARGER";
+			regulator-min-microamp = <90000>;
+			regulator-max-microamp = <950000>;
+			regulator-boot-on;
+		};
+	};
+};
+
+
+max77836 at 25 {
+	compatible = "maxim,max77836";
+	reg = <0x25>;
+	interrupt-parent = <&gpx1>;
+	interrupts = <5 IRQ_TYPE_NONE>;
+
+	muic: max77836-muic {
+		compatible = "maxim,max77836-muic";
+	};
+
+	regulators {
+		compatible = "maxim,max77836-regulator";
+
+		SAFEOUT {
+			regulator-name = "SAFEOUT";
+		};
+		CHARGER {
+			regulator-name = "CHARGER";
+			regulator-min-microamp = <90000>;
+			regulator-max-microamp = <950000>;
+			regulator-boot-on;
+		};
+		LDO1 {
+			regulator-name = "LDO1";
+			regulator-min-microvolt = <2700000>;
+			regulator-max-microvolt = <2700000>;
+		};
+		LDO2 {
+			regulator-name = "LDO2";
+			regulator-min-microvolt = <800000>;
+			regulator-max-microvolt = <3950000>;
+		};
+	};
+};
-- 
1.7.9.5

^ permalink raw reply related


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