* [PATCH v2] ARM: Drop fixed 200 Hz timer requirement from Samsung platforms
From: Sylwester Nawrocki @ 2016-11-18 10:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <22757093.ejshJp9T7L@wuerfel>
On 11/18/2016 09:46 AM, Arnd Bergmann wrote:
> On Friday, November 18, 2016 9:16:58 AM CET Krzysztof Kozlowski wrote:
>> > All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
>> > 200 Hz. Unfortunately in case of multiplatform image this affects also
>> > other platforms when Exynos is enabled.
>> >
>> > This looks like an very old legacy code, dating back to initial
>> > upstreaming of S3C24xx. Probably it was required for s3c24xx timer
>> > driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
>> > unused plat-samsung/time.c").
>> >
>> > Since then, this fixed 200 Hz spread everywhere, including out-of-tree
>> > Samsung kernels (SoC vendor's and Tizen's). I believe this choice
>> > was rather an effect of coincidence instead of conscious choice.
>> >
>> > Exynos uses its own MCT or arch timer and can work with all HZ values.
>> > Older platforms use newer Samsung PWM timer driver which should handle
>> > down to 100 Hz.
>> >
>> > Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
>> > A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
>> > other values.
>> >
>> > Reported-by: Lee Jones <lee.jones@linaro.org>
>> > [Dropping 200_HZ from S3C/S5P suggested by Arnd]
>> > Reported-by: Arnd Bergmann <arnd@arndb.de>
>> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>> > Cc: Kukjin Kim <kgene@kernel.org>
>> > Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
>> >
> Acked-by: Arnd Bergmann <arnd@arndb.de>
>
> Maybe add a paragraph about the specific problem:
>
> "On s3c24xx, the PWM counter is only 16 bit wide, and with the
> typical 12MHz input clock that overflows every 5.5ms. This works
> with HZ=200 or higher but not with HZ=100 which needs a 10ms
> interval between ticks. On Later chips (S3C64xx, S5P and EXYNOS),
> the counter is 32 bits and does not have this problem.
> The new samsung_pwm_timer driver solves the problem by scaling
> the input clock by a factor of 50 on s3c24xx, which makes it
> less accurate but allows HZ=100 as well as CONFIG_NO_HZ with
> fewer wakeups".
I've tested on S3C2440 SoC based board and I didn't notice any
issues with HZ=100.
Clock frequencies look a bit different because AFAIU MPLL
clock is mostly used as a root clock. The 12 MHz oscillator clock
is used a root clock for the MPLL.
refclk: 12000 kHz
mpll: 405000 kHz
upll: 48000 kHz
fclk: 405000 kHz
hclk: 101250 kHz
pclk: 50625 kHz
So frequency of the timer block's source clock (PCLK) is 50.625 MHz.
This is further divided by 50 in the prescaler as you pointed out.
So the 16-bit is clocked with 1012500 Hz clock. I added some printks
to verify this.
Here is boot log for HZ=200: http://pastebin.com/JuWZdYwh
and HZ=100 http://pastebin.com/HnDnBfhc
samsung_clocksource_init:351 pclk: 50625000, timer clock_rate: 1012500
sched_clock: 16 bits at 1012kHz, resolution 987ns, wraps every 32362962ns
I just don't understand why the log says timer overflow is every 32.362 ms
and not twice this value (65536 * 1/1012500).
--
Thanks,
Sylwester
^ permalink raw reply
* arasan,sdhci.txt "compatibility" DT binding
From: Rameshwar Sahu @ 2016-11-18 10:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <582ED9E0.5090506@free.fr>
Hi Mason,
Suman/APM added "arasan,sdhci-4.9a" in 308f3f8d8112
@Suman, @Rameshwar: what specific IP block does your SoC embed?
What does 4.9a refer to? The documentation revision number?
We have Arasan SD3.0/ SDIO3.0/ eMMC4.41 AHB Host Controller IP
embedded in our SoC and 4.9a is documentation revision number which
was given by arasan.
FYI this documentation date was May, 2012.
Thanks,
Ram
On Fri, Nov 18, 2016 at 4:07 PM, Mason <slash.tmp@free.fr> wrote:
> On 03/02/2016 16:33, Mason wrote:
>> On 03/02/2016 16:21, S?ren Brinkmann wrote:
>>> On Wed, 2016-02-03 at 10:58:24 +0100, Michal Simek wrote:
>>>> On 3.2.2016 09:31, Mason wrote:
>>>>> On 03/02/2016 08:20, Michal Simek wrote:
>>>>>
>>>>>> On 3.2.2016 03:33, Shawn Lin wrote:
>>>>>>> + Michal, S?ren Brinkmann
>>>>>>>
>>>>>>> On 2016/2/2 17:49, Mason wrote:
>>>>>>>> Hello everyone,
>>>>>>>>
>>>>>>>> Documentation/devicetree/bindings/mmc/arasan,sdhci.txt states:
>>>>>>>>
>>>>>>>> Required Properties:
>>>>>>>> - compatible: Compatibility string. Must be 'arasan,sdhci-8.9a' or
>>>>>>>> 'arasan,sdhci-4.9a' or 'arasan,sdhci-5.1'
>>>>>>>>
>>>>>>>> What do 8.9a, 4.9a, and 5.1 refer to?
>>>>>>>>
>>>>>>>
>>>>>>> Good question.
>>>>>>>
>>>>>>> Michal told me that 8.9a and 4.9a came from Xilinx
>>>>>>> databook which define their available arasan controller to be version
>>>>>>> 4.9a and 8.9a.
>>>>>>
>>>>>> Our version is coming from here.
>>>>>> http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf
>>>>>> page 28
>>>>>>
>>>>>> Datasheets from 2012 and 2010 doesn't look too recent and I expect that
>>>>>> Arasan did a lot of work from that time that's why I am not surprised
>>>>>> that you are not able to see that versions.
>>>>>
>>>>> Hello Michal,
>>>>>
>>>>> I'm even more confused.
>>>>>
>>>>> Arasan's 2010-02-19 data sheet is titled
>>>>> SD3.0 / SDIO3.0 / eMMC4.4 AHB Host Controller
>>>>>
>>>>> Page 28 of the Xilinx data sheet mentions
>>>>> SD2.0 / SDIO2.0 / MMC3.31 AHB Host Controller
>>>>> Version 8.9A_apr02nd_2010
>>>>>
>>>>> It does not make sense that Arasan would support SD3.0 in February,
>>>>> then go back to SD 2.0 in April.
>>>>>
>>>>> I do note eMMC4.4 vs MMC3.31 => perhaps these are two *different*
>>>>> IP blocks?
>>>>>
>>>>> Do your data sheets come with revision history?
>>>>
>>>> I don't have datasheet for this IP in my hand that's why I can't check it.
>>>> But I can't see any problem with it. Our zynq SoC supports SD2.0 and it
>>>> was requirement at that time. Bugs can happen. Arasan fixed it and
>>>> create new version.
>>>> At the same time can have 3.0 versions but vendor is just decide not to
>>>> use it for whatever reason.
>>>>
>>>> That's why timing of features and versions can upgrade any time and
>>>> unfortunately bugs happen.
>>>
>>> We have several Arasan data sheets here. The document names are:
>>> "SD2.0 / SDIO2.0 / MMC3.31 AHB Userguide" and the revisions are 9.2a,
>>> 4.4a and 5.4a. I have the feeling that the document revisions have
>>> been mistaken as the IP revision. I cannot find any other indicator
>>> for the IP revision though. Does the IP have a way to discover its
>>> revision?
>>
>> To summarize, it looks to me like
>> 4.9a and 8.9a are documentation revision numbers for this IP:
>> "SD2.0 / SDIO2.0 / MMC3.31 AHB Host Controller"
>>
>> and 5.1 seems to be the eMMC standard, so one of these IP:
>> http://arasan.com/products/emmc51/sd3-emmc-5-1-host/
>> http://arasan.com/products/emmc51/emmc-5-1-sd-4-1-host/
>>
>> Whereas my board is using still another IP:
>> "SD3.0 / SDIO3.0 / eMMC4.4 AHB Host Controller"
>>
>>
>> If that is correct, then I should be able to use the 8.9a
>> compatible string, and perhaps my hardware will work in
>> slightly degraded performance mode, from not using the
>> newest protocol gizmos available.
>
> Resurrecting this thread after a chat with Michal on IRC.
>
> The driver now supports a few more compatible strings.
> Tracing the history...
>
> Soren/Xilinx defined "arasan,sdhci-8.9a" in e3ec3a3d11ad
> They have the "SD2.0/SDIO2.0/MMC3.31 AHB Host Controller" version
> (8.9a might be a document revision number, dated 2010-04-02)
>
> Suman/APM added "arasan,sdhci-4.9a" in 308f3f8d8112
> @Suman, @Rameshwar: what specific IP block does your SoC embed?
> What does 4.9a refer to? The documentation revision number?
>
> Shawn/Rockchip added "arasan,sdhci-5.1" in da795ec26e25
> This seems to be for an *actual* eMMC 5.1 version
> (instead of a documentation version) as mentioned in
> the commit message.
>
> Douglas added "rockchip,rk3399-sdhci-5.1"
> and made several other improvements.
>
> I have reached out to Arasan support. Hopefully they can also
> help clear up the confusion, assuming they care about the
> Linux driver.
>
> Regards.
>
^ permalink raw reply
* [RFC 5/6] ARM: dts: dra7: add vivante for bb2d module
From: Lucas Stach @ 2016-11-18 10:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118024436.13447-5-robertcnelson@gmail.com>
Am Donnerstag, den 17.11.2016, 20:44 -0600 schrieb Robert Nelson:
> Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
> CC: Julien <jboulnois@gmail.com>
> CC: Nishanth Menon <nm@ti.com>
> CC: Tomi Valkeinen <tomi.valkeinen@ti.com>
> CC: Tony Lindgren <tony@atomide.com>
> ---
> arch/arm/boot/dts/dra7.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index 43488b6..22bd0a5 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -960,7 +960,7 @@
> };
>
> bb2d: bb2d at 59000000 {
> - compatible = "ti,dra7-bb2d";
> + compatible = "ti,dra7-bb2d","vivante,gc";
This is what the driver expects as a compatible, but it's not consistent
with the DT documentation patch you sent. As the driver can work out the
HW version from the identification registers I would say fix your DT
binding to only require "vivante,gc"
Regards,
Lucas
^ permalink raw reply
* [RFC 4/6] ARM: dts: dra7: add entry for bb2d module
From: Lucas Stach @ 2016-11-18 10:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118024436.13447-4-robertcnelson@gmail.com>
Am Donnerstag, den 17.11.2016, 20:44 -0600 schrieb Robert Nelson:
> From: Gowtham Tammana <g-tammana@ti.com>
>
> BB2D entry is added to the dts file. Crossbar index number is used
> for interrupt mapping.
>
> Signed-off-by: Gowtham Tammana <g-tammana@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> arch/arm/boot/dts/dra7.dtsi | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index addb753..43488b6 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -959,6 +959,16 @@
> ti,hwmods = "dmm";
> };
>
> + bb2d: bb2d at 59000000 {
> + compatible = "ti,dra7-bb2d";
> + reg = <0x59000000 0x0700>;
> + interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
> + ti,hwmods = "bb2d";
> + clocks = <&dpll_core_h24x2_ck>;
> + clock-names = "fclk";
"fclk" is not an accepted clock name for the etnaviv driver. It supports
up to 3 clocks: "bus", "core" and "shader". If there is only one clock
required in your design it would probably be the "core" clock.
Regards,
Lucas
^ permalink raw reply
* [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash
From: Krzeminski, Marcin (Nokia - PL/Wroclaw) @ 2016-11-18 10:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <DB6PR0401MB24074D231C8EBCC56E168CC989B00@DB6PR0401MB2407.eurprd04.prod.outlook.com>
> -----Original Message-----
> From: Yao Yuan [mailto:yao.yuan at nxp.com]
> Sent: Friday, November 18, 2016 5:20 AM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <marcin.krzeminski@nokia.com>; Han Xu <xhnjupt@gmail.com>
> Cc: David Woodhouse <dwmw2@infradead.org>; linux-
> kernel at vger.kernel.org; linux-mtd at lists.infradead.org;
> han.xu at freescale.com; Brian Norris <computersforpeace@gmail.com>;
> jagannadh.teki at gmail.com; linux-arm-kernel at lists.infradead.org
> Subject: RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family
> flash
>
> On Thu, Nov 17, 2016 at 10:14:55AM +0000, Krzeminski, Marcin (Nokia -
> PL/Wroclaw) wrote:
> > > On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin (Nokia
> > > -
> > > PL/Wroclaw) wrote:
> > > > > > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui
> > > > > > > <B56489@freescale.com>
> > > > > > > wrote:
> > > > > > > > From: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > >
> > > > > > > > With the physical sectors combination, S25FS-S family
> > > > > > > > flash requires some special operations for read/write functions.
> > > > > > > >
> > > > > > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > > ---
> > > > > > > > drivers/mtd/spi-nor/spi-nor.c | 56
> > > > > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > 1 file changed, 56 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb
> > > > > > > > 100644
> > > > > > > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > @@ -39,6 +39,10 @@
> > > > > > > >
> > > > > > > > #define SPI_NOR_MAX_ID_LEN 6
> > > > > > > > #define SPI_NOR_MAX_ADDR_WIDTH 4
> > > > > > > > +/* Added for S25FS-S family flash */
> > > > > > > > +#define SPINOR_CONFIG_REG3_OFFSET 0x800004
> > > > > > > > +#define CR3V_4KB_ERASE_UNABLE 0x8 #define
> > > > > > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC 0x81
> > > > > > > >
> > > > > > > > struct flash_info {
> > > > > > > > char *name;
> > > > > > > > @@ -78,6 +82,7 @@ struct flash_info { };
> > > > > > > >
> > > > > > > > #define JEDEC_MFR(info) ((info)->id[0])
> > > > > > > > +#define EXT_JEDEC(info) ((info)->id[5])
> > > > > > > >
> > > > > > > > static const struct flash_info *spi_nor_match_id(const
> > > > > > > > char *name);
> > > > > > > >
> > > > > > > > @@ -899,6 +904,7 @@ static const struct flash_info
> spi_nor_ids[] = {
> > > > > > > > */
> > > > > > > > { "s25sl032p", INFO(0x010215, 0x4d00, 64 * 1024,
> > > > > > > > 64,
> > > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > > { "s25sl064p", INFO(0x010216, 0x4d00, 64 * 1024,
> > > > > > > > 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > > + { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 *
> > > > > > > > + 1024, 512, 0)},
> > > > > > > > { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
> > > > > > > > { "s25fl256s1", INFO(0x010219, 0x4d01, 64 * 1024,
> > > > > > > > 512,
> > > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > > { "s25fl512s", INFO(0x010220, 0x4d00, 256 * 1024,
> > > > > > > > 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -
> 1036,6
> > > > > +1042,50
> > > > > > > @@ static const struct flash_info *spi_nor_read_id(struct
> > > > > > > spi_nor
> > > > > > > *nor)
> > > > > > > > return ERR_PTR(-ENODEV); }
> > > > > > > >
> > > > > > > > +/*
> > > > > > > > + * The S25FS-S family physical sectors may be configured
> > > > > > > > +as a
> > > > > > > > + * hybrid combination of eight 4-kB parameter sectors
> > > > > > > > + * at the top or bottom of the address space with all
> > > > > > > > + * but one of the remaining sectors being uniform size.
> > > > > > > > + * The Parameter Sector Erase commands (20h or 21h) must
> > > > > > > > + * be used to erase the 4-kB parameter sectors individually.
> > > > > > > > + * The Sector (uniform sector) Erase commands (D8h or
> > > > > > > > +DCh)
> > > > > > > > + * must be used to erase any of the remaining
> > > > > > > > + * sectors, including the portion of highest or lowest
> > > > > > > > +address
> > > > > > > > + * sector that is not overlaid by the parameter sectors.
> > > > > > > > + * The uniform sector erase command has no effect on
> > > > > > > > +parameter
> > > > > > > sectors.
> > > > > > > > + */
> > > > > > > > +static int spansion_s25fs_disable_4kb_erase(struct spi_nor
> *nor) {
> > > > > > > > + u32 cr3v_addr = SPINOR_CONFIG_REG3_OFFSET;
> > > > > > > > + u8 cr3v = 0x0;
> > > > > > > > + int ret = 0x0;
> > > > > > > > +
> > > > > > > > + nor->cmd_buf[2] = cr3v_addr >> 16;
> > > > > > > > + nor->cmd_buf[1] = cr3v_addr >> 8;
> > > > > > > > + nor->cmd_buf[0] = cr3v_addr >> 0;
> > > > > > > > +
> > > > > > > > + ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,
> > > &cr3v, 1);
> > > > > > > > + if (ret)
> > > > > > > > + return ret;
> > > > > > > > + if (cr3v & CR3V_4KB_ERASE_UNABLE)
> > > > > > > > + return 0;
> > > > > > > > + ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> > > > > > > > + if (ret)
> > > > > > > > + return ret;
> > > > > > > > + cr3v = CR3V_4KB_ERASE_UNABLE;
> > > > > > > > + nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
> > > > > > > > + nor->write(nor, cr3v_addr, 1, &cr3v);
> > > > > > > > +
> > > > > > > > + ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,
> > > &cr3v, 1);
> > > > > > > > + if (ret)
> > > > > > > > + return ret;
> > > > > > > > + if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> > > > > > > > + return -EPERM;
> > > > > > > > +
> > > > > > > > + return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > static int spi_nor_read(struct mtd_info *mtd, loff_t
> > > > > > > > from, size_t
> > > len,
> > > > > > > > size_t *retlen, u_char *buf) { @@
> > > > > > > > -1361,6
> > > > > > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const
> > > > > > > > +char *name,
> > > > > > > enum read_mode mode)
> > > > > > > > spi_nor_wait_till_ready(nor);
> > > > > > > > }
> > > > > > > >
> > > > > > > > + if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC)
> {
> > > > > > > > + ret = spansion_s25fs_disable_4kb_erase(nor);
> > > > > > > > + if (ret)
> > > > > > > > + return ret;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > if (!mtd->name)
> > > > > > > > mtd->name = dev_name(dev);
> > > > > > > > mtd->priv = nor;
> > > > > > > > --
> > > > > > > > 2.1.0.27.g96db324
> > > > > > > >
> > > > > > > >
> > > > > > > Hi Brian, I will ack this change but still feel it's kind of hacking code.
> > > > > > >
> > > > > > > Acked-by: Han xu <han.xu@nxp.com>
> > > > > >
> > > > > > I am new on the list so I am not sure if this topic has been discussed.
> > > > > > Generally our product functionality relay on those 4KiB sectors.
> > > > > > I know that this hack is already in u-boot, but if you
> > > > > > mainstream this you will force users of those 4KiB sectors to
> > > > > > do hack the
> > hack...
> > > > > > I believe the proper solution here is to use erase regions
> > > > > > functionality, I send and RFS about that some time ago.
> > > > >
> > > > > Do you mind to send me a link for reference?
> > > > >
> > > > Han,
> > > >
> > > > Sorry, It seem I have not posted erase region changes (only those
> > > > regarding DUAL/QUAD I/O).
> > > > Generally, in this flash you need to create 3 erase regions
> > > > (because in FS-S family support only 4KiB erase on parameters sector -
> eg.
> > > > 1.2.2.4 in
> > > S25FS512S).
> > > > In my case regions are:
> > > > 1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 - SE_CMD
> > > (0xd8/0xdc) 3.
> > > > Rest of the flash SE_CMD (0xd8/0xdc)
> > > >
> > > > To erase whole flash you can also use CHIP_ERASE_CMD (0x60/0xC7)
> > > > command, you just need to add one more mtd partition that will
> > > > cover
> > > whole flash.
> > > >
> > >
> > > Hi Krzeminski,
> > >
> > > Do you think is there any great advantages for enable 4KB?
> > > Because for NXP(Freescale) QSPI controller, there is only support
> > > max to 16 groups command.
> > >
> > > So It's hard to give 3 groups command just for erase (0x21,0xdc and 0xc7).
> > > So we have to disable the 4kb erase and only use 256kbytes in this patch.
> > >
> > Yes, if you disable parameters sector in spi-nor framework you will
> > disable it for all spi-nor clients not only for NXP QSPI controller.
> > There are users (at least me) that relay on parameters sector functionality.
> This patch will brake it.
> >
> > Thanks,
>
> Hi Krzeminski,
>
> Get it.
> So do you think how about that I add a flag in dts to select it?
> The user want's disable 4kb, he can add the flag.
>
> In spi-nor.c:
> if (of_property_read_bool(np, "spi-nor, disable-4kb")) {
> spansion_s25fs_disable_4kb_erase();
> }
> else
> ...
>
> In dts:
>
> &qspi {
> num-cs = <2>;
> bus-num = <0>;
> status = "okay";
>
> qflash0: s25fs512s at 0 {
> compatible = "spansion, s25fs512s";
> spi-nor, disable-4kb
> #address-cells = <1>;
> #size-cells = <1>;
> spi-max-frequency = <20000000>;
> reg = <0>;
> };
>
> I think it should be a better way.
>
> How about your think?
This looks much better - at least for me.
There are some parameters in JESD216 standard regarding parameters sector,
but unfortunately I have not investigated that. You can take a look at Cyrille series,
that adds support for JESD216 standard.
Thanks,
Marcin
>
> Thanks.
>
^ permalink raw reply
* [arm-soc:qcom/arm64 3/13] Error: arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi:12.20-21 syntax error
From: kbuild test robot @ 2016-11-18 11:01 UTC (permalink / raw)
To: linux-arm-kernel
Hi Srinivas,
FYI, the error/warning still remains.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git qcom/arm64
head: feeaf56ac78d283efe65ea60ec999d4bf3cf395e
commit: 50784e61032d89cbbc46ed73a5fb15f27940b947 [3/13] dts: arm64: db820c: add pmic pins specific dts file
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 50784e61032d89cbbc46ed73a5fb15f27940b947
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
>> Error: arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi:12.20-21 syntax error
FATAL ERROR: Unable to parse input tree
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 31923 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161118/f34100a1/attachment-0001.gz>
^ permalink raw reply
* System/uncore PMUs and unit aggregation
From: Jan Glauber @ 2016-11-18 11:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161117181708.GT22855@arm.com>
On Thu, Nov 17, 2016 at 06:17:08PM +0000, Will Deacon wrote:
> Hi all,
>
> We currently have support for three arm64 system PMUs in flight:
>
> [Cavium ThunderX] http://lkml.kernel.org/r/cover.1477741719.git.jglauber at cavium.com
> [Hisilicon Hip0x] http://lkml.kernel.org/r/1478151727-20250-1-git-send-email-anurup.m at huawei.com
> [Qualcomm L2] http://lkml.kernel.org/r/1477687813-11412-1-git-send-email-nleeder at codeaurora.org
>
> Each of which have to deal with multiple underlying hardware units in one
> way or another. Mark and I recently expressed a desire to expose these
> units to userspace as individual PMU instances, since this can allow:
>
> * Fine-grained control of events from userspace, when you want to see
> individual numbers as opposed to a summed total
>
> * Potentially ease migration to new SoC revisions, where the units
> are laid out slightly differently
>
> * Easier handling of cases where the units aren't quite identical
>
> however, this received pushback from all of the patch authors, so there's
> clearly a problem with this approach. I'm hoping we can try to resolve
> this here.
Good to know. Thanks for adressing this on a higher level.
> Speaking to Mark earlier today, we came up with the following rough rules
> for drivers that present multiple hardware units as a single PMU:
>
> 1. If the units share some part of the programming interface (e.g. control
> registers or interrupts), then they must be handled by the same PMU.
> Otherwise, they should be treated independently as separate PMU
> instances.
Can you elaborate why they should be treated independent in the later
case? What is the problem with going through a list and writing the
control register per unit?
> 2. If the units are handled by the same PMU, then care must be taken to
> handle event groups correctly. That is, if the units cannot be started
> and stopped atomically, cross-unit groups must be rejected by the
> driver. Furthermore, any cross-unit scheduling constraints must be
> honoured so that all the units targetted by a group can schedule the
> group concurrently.
>
> 3. Summing the counters across units is only permitted if the units
> can all be started and stopped atomically. Otherwise, the counters
> should be exposed individually. It's up to the driver author to
> decide what makes sense to sum.
Do you mean started/stopped atomically across units?
> 4. Unit topology can optionally be described in sysfs (we should pick
> some standard directory naming here), and then events targetting
> specific units can have the unit identifier extracted from the topology
> encoded in some configN fields.
>
> The million dollar question is: how does that fit in with the drivers I
> mentioned at the top? Is this overly restrictive, or have we missed stuff?
>
> We certainly want to allow flexibility in the way in which the drivers
> talk to the hardware, but given that these decisions directly affect the
> user ABI, some consistent ground rules are required.
>
> For Cavium ThunderX, it's not clear whether or not the individual units
> could be expressed as separate PMUs, or whether they're caught by one of
> the rules above. The Qualcomm L2 looks like it's doing the right thing
> and we can't quite work out what the Hisilicon Hip0x topology looks like,
> since the interaction with djtag is confusing.
On Cavium ThunderX the current patches add 4 PMU types, which unfortunately
are all handled different. The L2C-TAD and OCX-TLK have control
registers per unit. The LMC and L2C-CBC don't have control registers,
(free-running counters). So rule 1 might be too restrictive.
I've not looked into groups, would these allow to merge counters from
different PMUs in the kernel?
--Jan
> If the driver authors (on To:) could shed some light on this, then that
> would be much appreciated!
>
> Thanks,
>
> Will
^ permalink raw reply
* [PATCH v2] ARM: Drop fixed 200 Hz timer requirement from Samsung platforms
From: Krzysztof Kozlowski @ 2016-11-18 11:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8dfbb04f-d377-b51a-3010-481a5f977507@samsung.com>
On Fri, Nov 18, 2016 at 11:43:14AM +0100, Sylwester Nawrocki wrote:
> On 11/18/2016 09:46 AM, Arnd Bergmann wrote:
> > On Friday, November 18, 2016 9:16:58 AM CET Krzysztof Kozlowski wrote:
> >> > All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
> >> > 200 Hz. Unfortunately in case of multiplatform image this affects also
> >> > other platforms when Exynos is enabled.
> >> >
> >> > This looks like an very old legacy code, dating back to initial
> >> > upstreaming of S3C24xx. Probably it was required for s3c24xx timer
> >> > driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
> >> > unused plat-samsung/time.c").
> >> >
> >> > Since then, this fixed 200 Hz spread everywhere, including out-of-tree
> >> > Samsung kernels (SoC vendor's and Tizen's). I believe this choice
> >> > was rather an effect of coincidence instead of conscious choice.
> >> >
> >> > Exynos uses its own MCT or arch timer and can work with all HZ values.
> >> > Older platforms use newer Samsung PWM timer driver which should handle
> >> > down to 100 Hz.
> >> >
> >> > Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
> >> > A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
> >> > other values.
> >> >
> >> > Reported-by: Lee Jones <lee.jones@linaro.org>
> >> > [Dropping 200_HZ from S3C/S5P suggested by Arnd]
> >> > Reported-by: Arnd Bergmann <arnd@arndb.de>
> >> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> >> > Cc: Kukjin Kim <kgene@kernel.org>
> >> > Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
> >> >
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> >
> > Maybe add a paragraph about the specific problem:
> >
> > "On s3c24xx, the PWM counter is only 16 bit wide, and with the
> > typical 12MHz input clock that overflows every 5.5ms. This works
> > with HZ=200 or higher but not with HZ=100 which needs a 10ms
> > interval between ticks. On Later chips (S3C64xx, S5P and EXYNOS),
> > the counter is 32 bits and does not have this problem.
> > The new samsung_pwm_timer driver solves the problem by scaling
> > the input clock by a factor of 50 on s3c24xx, which makes it
> > less accurate but allows HZ=100 as well as CONFIG_NO_HZ with
> > fewer wakeups".
>
> I've tested on S3C2440 SoC based board and I didn't notice any
> issues with HZ=100.
>
> Clock frequencies look a bit different because AFAIU MPLL
> clock is mostly used as a root clock. The 12 MHz oscillator clock
> is used a root clock for the MPLL.
>
> refclk: 12000 kHz
> mpll: 405000 kHz
> upll: 48000 kHz
> fclk: 405000 kHz
> hclk: 101250 kHz
> pclk: 50625 kHz
>
> So frequency of the timer block's source clock (PCLK) is 50.625 MHz.
> This is further divided by 50 in the prescaler as you pointed out.
>
> So the 16-bit is clocked with 1012500 Hz clock. I added some printks
> to verify this.
>
> Here is boot log for HZ=200: http://pastebin.com/JuWZdYwh
> and HZ=100 http://pastebin.com/HnDnBfhc
>
> samsung_clocksource_init:351 pclk: 50625000, timer clock_rate: 1012500
> sched_clock: 16 bits at 1012kHz, resolution 987ns, wraps every 32362962ns
Thanks for tests! I really appreciate it.
> I just don't understand why the log says timer overflow is every 32.362 ms
> and not twice this value (65536 * 1/1012500).
The answer could be at kernel/time/clocksource.c:
* NOTE: This function includes a safety margin of 50%, in other words, we
* return half the number of nanoseconds the hardware counter can technically
* cover.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: zhichang.yuan @ 2016-11-18 11:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2436881.9RqUVYxmDf@wuerfel>
Hi, Arnd,
On 2016/11/18 17:20, Arnd Bergmann wrote:
> On Friday, November 11, 2016 6:07:07 PM CET zhichang.yuan wrote:
>>
>> I have similar idea as your PPC MMIO.
>>
>> We notice the prototype of {in/out()} is something like that:
>>
>> static inline u8 inb(unsigned long addr)
>> static inline void outb(u8 value, unsigned long addr)
>>
>> The type of parameter 'addr' is unsigned long. For I/O space, it is big enough.
>> So, could you divide this 'addr' into several bit segments? The top 8 bits is
>> defined as bus index. For normal direct IO, the bus index is 0. For those bus
>> device which need indirectIO or some special I/O accessors, when these devices
>> are initializing, can request to allocate an unique ID to them, and register
>> their own accessors to the entry which is corresponding to the ID.
>
> Ah, have you looked at the IA64 code? It does exactly this.
> For ARM64 we decided to use the same basic approach as powerpc with
> a single range of virtual memory for mapping it as that somewhat
> simplified all cases we knew about at the time.
Yes. I spent some time to trace how to work on PPC. But the code is a bit long,
I am not clear on how the indirectIO there was supported.
I noticed there are CONFIG_PPC_INDIRECT_PIO and CONFIG_PPC_INDIRECT_MMIO on PPC.
It seems that only CONFIG_PPC_INDIRECT_MMIO applied some MSB to store the bus
tokens which are used to get iowa_busses[] for specific operation helpers.
I can not find how CONFIG_PPC_INDIRECT_PIO support multiple ISA domains. It
seems only Opal-lpc.c adopt this INDIRECT_PIO method.
Although CONFIG_PPC_INDIRECT_MMIO is for MMIO, seems not suitable for ISA/LPC
I/O. But this idea is helpful.
what else did I miss??
>
>> In this way, we can support multiple domains, I think.
>> But I am not sure whether it is feasible, for example, are there some
>> architectures/platforms had populated the top 8 bits? Do we need to request IO
>> region from ioport_resource for those devices? etc...
>
> On a 64-bit architecture, the top 32 bits of the port number are
> definitely free to use for this, and 8 bits are probably sufficient.
>
> Even on 32 bit architectures, I can't see why we'd ever need more than
> 16 bits worth of addressing within a domain, so using 8 bit domain
> and 16 bit address leaves 8 or 40 unused bits.
Yes. 8 bits are enough.
But the maximal PIO on some architectures are defined as ~0 or -1. There is no
any bare space left. Probably we can not ensure the upper 8 bits available.
Thanks,
Zhichang
>
> Arnd
>
> .
>
^ permalink raw reply
* [PATCH v3] ARM: Drop fixed 200 Hz timer requirement from Samsung platforms
From: Krzysztof Kozlowski @ 2016-11-18 11:15 UTC (permalink / raw)
To: linux-arm-kernel
All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
200 Hz. Unfortunately in case of multiplatform image this affects also
other platforms when Exynos is enabled.
This looks like an very old legacy code, dating back to initial
upstreaming of S3C24xx. Probably it was required for s3c24xx timer
driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
unused plat-samsung/time.c").
Since then, this fixed 200 Hz spread everywhere, including out-of-tree
Samsung kernels (SoC vendor's and Tizen's). I believe this choice
was rather an effect of coincidence instead of conscious choice.
On S3C24xx, the PWM counter is only 16 bit wide, and with the
typical 12MHz input clock that overflows every 5.5ms. This works
with HZ=200 or higher but not with HZ=100 which needs a 10ms
interval between ticks. On Later chips (S3C64xx, S5P and EXYNOS),
the counter is 32 bits and does not have this problem.
The new samsung_pwm_timer driver solves the problem by scaling the input
clock by a factor of 50 on S3C24xx, which makes it less accurate but
allows HZ=100 as well as CONFIG_NO_HZ with fewer wakeups.
Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
other values.
Reported-by: Lee Jones <lee.jones@linaro.org>
[Dropping of 200_HZ from S3C/S5P was suggested by Arnd]
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Kukjin Kim <kgene@kernel.org>
[Tested on Exynos5800]
Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Kukjin Kim <kgene@kernel.org>
[Tested on S3C2440]
Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
Changes since v2:
1. Extend message.
2. Add Kukjin's ack.
3. Add Sylwester's tested-by.
Changes since v1:
1. Add Javier's tested-by.
2. Drop HZ_FIXED also from ARCH_S5PV210 and ARCH_S3C24XX after Arnd
suggestions and analysis.
---
arch/arm/Kconfig | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b5d529fdffab..ced2e08a9d08 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1496,8 +1496,7 @@ source kernel/Kconfig.preempt
config HZ_FIXED
int
- default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
- ARCH_S5PV210 || ARCH_EXYNOS4
+ default 200 if ARCH_EBSA110
default 128 if SOC_AT91RM9200
default 0
--
2.7.4
^ permalink raw reply related
* [arm-soc:qcom/arm64 11/13] arch/arm64/boot/dts/qcom/msm8992.dtsi:14:48: fatal error: dt-bindings/clock/qcom,gcc-msm8994.h: No such file or directory
From: kbuild test robot @ 2016-11-18 11:30 UTC (permalink / raw)
To: linux-arm-kernel
tree: https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git qcom/arm64
head: feeaf56ac78d283efe65ea60ec999d4bf3cf395e
commit: 6a6d1978f9c0d818b4370903e1f3eecf1681c932 [11/13] arm64: dts: msm8992 SoC and LG Bullhead (Nexus 5X) support
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 6a6d1978f9c0d818b4370903e1f3eecf1681c932
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
In file included from arch/arm64/boot/dts/qcom/msm8992-bullhead-rev-101.dts:16:0:
>> arch/arm64/boot/dts/qcom/msm8992.dtsi:14:48: fatal error: dt-bindings/clock/qcom,gcc-msm8994.h: No such file or directory
#include <dt-bindings/clock/qcom,gcc-msm8994.h>
^
compilation terminated.
vim +14 arch/arm64/boot/dts/qcom/msm8992.dtsi
8 * but WITHOUT ANY WARRANTY; without even the implied warranty of
9 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
10 * GNU General Public License for more details.
11 */
12
13 #include <dt-bindings/interrupt-controller/arm-gic.h>
> 14 #include <dt-bindings/clock/qcom,gcc-msm8994.h>
15
16 / {
17 model = "Qualcomm Technologies, Inc. MSM 8992";
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 31923 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161118/3411caec/attachment-0001.gz>
^ permalink raw reply
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Arnd Bergmann @ 2016-11-18 11:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <582EE223.4090605@hisilicon.com>
On Friday, November 18, 2016 7:12:35 PM CET zhichang.yuan wrote:
> Hi, Arnd,
>
>
> On 2016/11/18 17:20, Arnd Bergmann wrote:
> > On Friday, November 11, 2016 6:07:07 PM CET zhichang.yuan wrote:
> >>
> >> I have similar idea as your PPC MMIO.
> >>
> >> We notice the prototype of {in/out()} is something like that:
> >>
> >> static inline u8 inb(unsigned long addr)
> >> static inline void outb(u8 value, unsigned long addr)
> >>
> >> The type of parameter 'addr' is unsigned long. For I/O space, it is big enough.
> >> So, could you divide this 'addr' into several bit segments? The top 8 bits is
> >> defined as bus index. For normal direct IO, the bus index is 0. For those bus
> >> device which need indirectIO or some special I/O accessors, when these devices
> >> are initializing, can request to allocate an unique ID to them, and register
> >> their own accessors to the entry which is corresponding to the ID.
> >
> > Ah, have you looked at the IA64 code? It does exactly this.
> > For ARM64 we decided to use the same basic approach as powerpc with
> > a single range of virtual memory for mapping it as that somewhat
> > simplified all cases we knew about at the time.
>
> Yes. I spent some time to trace how to work on PPC. But the code is a bit long,
> I am not clear on how the indirectIO there was supported.
>
> I noticed there are CONFIG_PPC_INDIRECT_PIO and CONFIG_PPC_INDIRECT_MMIO on PPC.
> It seems that only CONFIG_PPC_INDIRECT_MMIO applied some MSB to store the bus
> tokens which are used to get iowa_busses[] for specific operation helpers.
> I can not find how CONFIG_PPC_INDIRECT_PIO support multiple ISA domains. It
> seems only Opal-lpc.c adopt this INDIRECT_PIO method.
>
> Although CONFIG_PPC_INDIRECT_MMIO is for MMIO, seems not suitable for ISA/LPC
> I/O. But this idea is helpful.
>
> what else did I miss??
I mentioned two different things here: ia64 IIRC uses some bits of the
port number to look up the domain, while powerpc traditionally had no
support for any such lookup, it did the same thing as ARM64 with
virtual remapping of MMIO ranges into an address range starting at
a fixed virtual address.
CONFIG_PPC_INDIRECT_PIO is a fairly recent addition, I was not thinking
of that.
> >> In this way, we can support multiple domains, I think.
> >> But I am not sure whether it is feasible, for example, are there some
> >> architectures/platforms had populated the top 8 bits? Do we need to request IO
> >> region from ioport_resource for those devices? etc...
> >
> > On a 64-bit architecture, the top 32 bits of the port number are
> > definitely free to use for this, and 8 bits are probably sufficient.
> >
> > Even on 32 bit architectures, I can't see why we'd ever need more than
> > 16 bits worth of addressing within a domain, so using 8 bit domain
> > and 16 bit address leaves 8 or 40 unused bits.
>
> Yes. 8 bits are enough.
> But the maximal PIO on some architectures are defined as ~0 or -1. There is no
> any bare space left. Probably we can not ensure the upper 8 bits available.
Right, we clearly can't use it across all architectures. The trick with
architectures using ULONG_MAX as the limit for port numbers is that they
treat it as a 1:1 mapping between port numbers and virtual addresses,
which is yet another way to handle the MMIO-based devices, but that has
a number of downsides we don't need to get into now.
What I think the code should do is a generic workaround handling that
architectures can opt-in to. We'd start doing this on ARM64 only,
and can then decide whether to change ARM or PowerPC over to use
that as well.
Arnd
^ permalink raw reply
* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-11-18 11:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <582469C8.9090609@hisilicon.com>
[found this old mail in my drafts folder, might as well send it now]
On Thursday, November 10, 2016 8:36:24 PM CET zhichang.yuan wrote:
> Sorry! I can't catch your idea yet:(
>
> When to register the I/O range? Is it done just after the successfully
> of_translate_address() during the children scanning?
No, you do it when first finding the bus itself, just like we do for
PCI host bridges.
> If yes, when a child is scanning, there is no range data in arm64_extio_ops. The
> addr_is_indirect_io() calling in of_get_isa_indirect_io() don't need. All we can
> check is just whether the address to be translated is IO and is under a parent
> device which has no 'ranges' property.
The children should only be scanned after the I/O range has been
registered for the parent.
> > Your current version has
> >
> > if (arm64_extio_ops->pfout) \
> > arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> > addr, value, sizeof(type)); \
> >
> > Instead, just subtract the start of the range from the logical
> > port number to transform it back into a bus-local port number:
> >
> > if (arm64_extio_ops->pfout) \
> > arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> > addr - arm64_extio_ops->start, value, sizeof(type)); \
> >
> I think there is some information needed sync.
> In the old patch-set, we don't bypass the pci_address_to_pio() after
> successfully of_translate_address(). In this way, we don't need to reserve any
> PIO space for our LPC since the logical port are from the same mapping
> algorithm. Based on this way, the port number in the device resource is logical
> one, then we need to subtract the start of the resource to get back the
> bus-local port.
>
> From V3, we don't apply the mapping based on pci_address_to_pio(), the
> of_translate_address() return the bus-local port directly and store into
> relevant device resource. So, in the current arm64_extio_ops->pfout(), the
> reverse translation don't need anymore. The input "addr" is bus-local port now.
Ok, so this would have to be changed again: If we want to support multiple
bus domains, of_translate_address() must translate between the bus specific
address and the general Linux I/O port number. Even without doing that,
it seems nicer to not overlap the range of the first PCI host bridge.
Arnd
^ permalink raw reply
* [GIT PULL 2/3] ARM64: dts: exynos: DT for v4.10
From: Krzysztof Kozlowski @ 2016-11-18 11:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118015954.GP8578@localhost>
On Thu, Nov 17, 2016 at 05:59:54PM -0800, Olof Johansson wrote:
> On Tue, Nov 08, 2016 at 08:26:29PM +0200, Krzysztof Kozlowski wrote:
> > Hi,
> >
> > Exynos5433 + two boards using it. Mobile boards! :)
> >
> > I am really happy to push it. I know that it has been a lot of effort
> > in Samsung to mainline this.
> >
> > Best regards,
> > Krzysztof
> >
> >
> > The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
> >
> > Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
> >
> > are available in the git repository at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git tags/samsung-dt64-4.10
> >
> > for you to fetch changes up to 8ac46fc57df82efbc19194dddd335b6c7a960c31:
> >
> > arm64: dts: exynos: Add dts file for Exynos5433-based TM2E board (2016-11-03 22:19:57 +0200)
> >
> > ----------------------------------------------------------------
> > Finally, I am really pleased to announce adding support for Exynos5433 ARMv8
> > SoC along with two boards. A lot of Samsung people contributed into this
> > but the final work and commits were done by Chanwoo Choi.
> >
> > This means that for v4.10 we got:
> > 1. Exynos5433 DTSI.
> > 2. Two boards: TM2 and TM2E. These are (almost fully) working mobile phones.
>
> Awesome! Looks like TM2 is a Tizen reference board? Great to see the support,
> even if it's taken a while.
Yes, I think these were planned to be reference devices. Userspace
packages and snapshots are published for some quite long time
(http://download.tizen.org/snapshots/tizen/3.0-mobile/latest/images/arm64-wayland/).
Best regards,
Krzysztof
^ permalink raw reply
* [GIT PULL v2] firmware: SCPI updates for v4.10
From: Sudeep Holla @ 2016-11-18 11:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161028112920.GB12241@e107155-lin>
Hi ARM-SoC Team,
I have decoupled the platform specific binding from generic SCPI. Also
I have renamed "arm,legacy-scpi" to "arm,scpi-pre-1.0". Since I haven't
heard back any objections from Olof/Rob for my response, I am sending
the pull request now.
--
Regards,
Sudeep
The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git tags/scpi-updates-4.10
for you to fetch changes up to 8358c6b5fc8c160d0af8654f313b8a7745f8e304:
firmware: arm_scpi: add support for pre-v1.0 SCPI compatible (2016-11-17 16:31:20 +0000)
----------------------------------------------------------------
SCPI updates for v4.10
1. Adds support for pre-v1.0 SCPI protocol versions
2. Adds support for SCPI used on Amlogic GXBB SoC platforms using the
newly added pre-v1.0 SCPI protocol
3. Decouples some platform specific details from generic SCPI binding
----------------------------------------------------------------
Neil Armstrong (4):
firmware: arm_scpi: increase MAX_DVFS_OPPS to 16 entries
firmware: arm_scpi: add alternative legacy structures, functions and macros
firmware: arm_scpi: allow firmware with get_capabilities not implemented
Documentation: bindings: Add support for Amlogic GXBB SCPI protocol
Sudeep Holla (4):
firmware: arm_scpi: add command indirection to support legacy commands
Documentation: bindings: decouple juno specific details from generic binding
Documentation: bindings: add compatible specific to pre v1.0 SCPI protocols
firmware: arm_scpi: add support for pre-v1.0 SCPI compatible
.../devicetree/bindings/arm/amlogic,scpi.txt | 20 ++
Documentation/devicetree/bindings/arm/arm,scpi.txt | 25 +-
.../devicetree/bindings/arm/juno,scpi.txt | 26 ++
drivers/firmware/arm_scpi.c | 276 ++++++++++++++++++---
4 files changed, 301 insertions(+), 46 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/amlogic,scpi.txt
create mode 100644 Documentation/devicetree/bindings/arm/juno,scpi.txt
^ permalink raw reply
* [PATCH 15/20] ARM/hw_breakpoint: Convert to hotplug state machine
From: Will Deacon @ 2016-11-18 12:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161117183541.8588-16-bigeasy@linutronix.de>
On Thu, Nov 17, 2016 at 07:35:36PM +0100, Sebastian Andrzej Siewior wrote:
> Install the callbacks via the state machine and let the core invoke
> the callbacks on the already online CPUs.
>
> smp_call_function_single() has been removed because the function is already
> invoked on the target CPU.
>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> arch/arm/kernel/hw_breakpoint.c | 44 ++++++++++++++++++-----------------------
> 1 file changed, 19 insertions(+), 25 deletions(-)
[...]
> static int __init arch_hw_breakpoint_init(void)
> {
> + int ret;
> +
> debug_arch = get_debug_arch();
>
> if (!debug_arch_supported()) {
> @@ -1072,8 +1069,6 @@ static int __init arch_hw_breakpoint_init(void)
> core_num_brps = get_num_brps();
> core_num_wrps = get_num_wrps();
>
> - cpu_notifier_register_begin();
> -
> /*
> * We need to tread carefully here because DBGSWENABLE may be
> * driven low on this core and there isn't an architected way to
> @@ -1082,15 +1077,18 @@ static int __init arch_hw_breakpoint_init(void)
> register_undef_hook(&debug_reg_hook);
>
> /*
> - * Reset the breakpoint resources. We assume that a halting
> - * debugger will leave the world in a nice state for us.
> + * Register CPU notifier which resets the breakpoint resources. We
> + * assume that a halting debugger will leave the world in a nice state
> + * for us.
> */
> - on_each_cpu(reset_ctrl_regs, NULL, 1);
> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "arm/hw_breakpoint:online",
> + dbg_reset_online, NULL);
I'm slightly unsure about this. The dbg_reset_online callback can execute
undefined instructions (unfortunately, there's no way to probe the presence
of some of the debug registers), so it absolutely has to run within the
register_undef_hook/unregister_undef_hook calls that are in this function.
With this patch, I worry that the callback can be postponed to ONLINE time
for other CPUs, and then the kernel will panic.
Will
^ permalink raw reply
* [PATCH v3 06/13] ARM: dts: armada-375: Fixup sa-ram DT warning
From: Gregory CLEMENT @ 2016-11-18 12:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118095925.770496c3@free-electrons.com>
Hi Thomas,
On ven., nov. 18 2016, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Fri, 18 Nov 2016 00:08:23 +0100, Gregory CLEMENT wrote:
>
>> - crypto_sram0: sa-sram0 {
>> + /* The following unit addresses (for sa-sram) are composed of
>> + * the target value (bit [40-47]), attributes value (bits
>> + * [32-39], and the address value in the window memory: [0-31].
>> + */
>
> The "address value in the window memory" part doesn't make a lot of
> sense. Maybe:
>
> "The following unit addresses are composed of the window target ID
> (bits 40-47), the window target attributes (bits 32-39) and the offset
> inside the window."
I'm fine with it
>
> Also, the comment formatting is not compliant with the coding style,
> should be:
>
> /*
> * ...
> * ...
> */
>
Hum yes I need to teach emacs how to properly format the comments.
> But do we really want this comment above each node? Couldn't we instead
> add this explanation in the mvebu-mbus.txt DT binding?
We could but I fear that nobody will read it.
Indeed if you know that in order to understand the unit address, you will
have to have a look an the binding of the mvebu-mbus, then it means that
you already are an expert and actually you barely need to read it!
In order to have less change we could at least put it near the MBUS_ID
macro and if the mvebu-mbus.txt DT binding too.
Gregory
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH] arm: spin one more cycle in timer-based delays
From: Will Deacon @ 2016-11-18 12:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <582B0F61.6030903@free.fr>
On Tue, Nov 15, 2016 at 02:36:33PM +0100, Mason wrote:
> When polling a tick counter in a busy loop, one might sample the
> counter just *before* it is updated, and then again just *after*
> it is updated. In that case, while mathematically v2-v1 equals 1,
> only epsilon has really passed.
>
> So, if a caller requests an N-cycle delay, we spin until v2-v1
> is strictly greater than N to avoid these random corner cases.
>
> Signed-off-by: Mason <slash.tmp@free.fr>
> ---
> arch/arm/lib/delay.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c
> index 69aad80a3af4..3f1cd15ca102 100644
> --- a/arch/arm/lib/delay.c
> +++ b/arch/arm/lib/delay.c
> @@ -60,7 +60,7 @@ static void __timer_delay(unsigned long cycles)
> {
> cycles_t start = get_cycles();
>
> - while ((get_cycles() - start) < cycles)
> + while ((get_cycles() - start) <= cycles)
> cpu_relax();
> }
I thought a bit about this last night. It is well known that the delay
routines are not guaranteed to be accurate, and I don't *think* that's
limited to over-spinning, so arguably this isn't a bug. However, taking
the approach that "drivers should figure it out" is also unhelpful,
because the frequency of the underlying counter isn't generally known
and so drivers can't simply adjust the delay parameter.
If you want to go ahead with this change, I do think that you should fix
the other architectures for consistency (particularly arm64, which is
likely to share drivers with arch/arm/). However, given that I don't
think you've seen a failure in practice, it might be a hard sell to get
the patches picked up, especially given the deliberately vague guarantees
offered by the delay loop.
Will
^ permalink raw reply
* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Gabriele Paoloni @ 2016-11-18 12:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2281986.miqFuFkAbO@wuerfel>
Hi Arnd many thanks for your help here
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 18 November 2016 10:18
> To: liviu.dudau at arm.com
> Cc: Gabriele Paoloni; linux-arm-kernel at lists.infradead.org;
> Yuanzhichang; mark.rutland at arm.com; devicetree at vger.kernel.org;
> lorenzo.pieralisi at arm.com; minyard at acm.org; linux-pci at vger.kernel.org;
> benh at kernel.crashing.org; John Garry; will.deacon at arm.com; linux-
> kernel at vger.kernel.org; xuwei (O); Linuxarm; zourongrong at gmail.com;
> robh+dt at kernel.org; kantyzc at 163.com; linux-serial at vger.kernel.org;
> catalin.marinas at arm.com; olof at lixom.net; bhelgaas at go ogle.com;
> zhichang.yuan02 at gmail.com; Jason Gunthorpe; Thomas Petazzoni
> Subject: Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on
> Hip06
>
> On Monday, November 14, 2016 11:26:25 AM CET liviu.dudau at arm.com wrote:
> > On Mon, Nov 14, 2016 at 08:26:42AM +0000, Gabriele Paoloni wrote:
> > > > Nope, that is not what it means. It means that PCI devices can
> see I/O
> > > > addresses
> > > > on the bus that start from 0. There never was any usage for non-
> PCI
> > > > controllers
> > >
> > > So I am a bit confused...
> > > From http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> > > It seems that ISA buses operate on cpu I/O address range [0,
> 0xFFF].
> > > I thought that was the reason why for most architectures we have
> > > PCIBIOS_MIN_IO equal to 0x1000 (so I thought that ISA controllers
> > > usually use [0, PCIBIOS_MIN_IO - 1] )
> >
> > First of all, cpu I/O addresses is an x86-ism. ARM architectures and
> others
> > have no separate address space for I/O, it is all merged into one
> unified
> > address space. So, on arm/arm64 for example, PCIBIOS_MIN_IO = 0 could
> mean
> > that we don't care about ISA I/O because the platform does not
> support having
> > an ISA bus (e.g.).
>
> I think to be more specific, PCIBIOS_MIN_IO=0 would indicate that you
> cannot
> have a PCI-to-ISA or PCI-to-LPC bridge in any PCI domain. This is
> different
> from having an LPC master outside of PCI, as that lives in its own
> domain
> and has a separately addressable I/O space.
Yes correct so if we go for the single domain solution arch that
have PCIBIOS_MIN_IO=0 cannot support special devices such as LPC
unless we also redefine PCIBIOS_MIN_IO, right?
>
> > > As said before this series forbid IO tokens to be in [0,
> PCIBIOS_MIN_IO)
> > > to allow special ISA controllers to use that range with special
> > > accessors.
> > > Having a variable threshold would make life much more difficult
> > > as there would be a probe dependency between the PCI controller and
> > > the special ISA one (PCI to wait for the special ISA device to be
> > > probed and set the right threshold value from DT or ACPI table).
> > >
> > > Instead using PCIBIOS_MIN_IO is easier and should not impose much
> > > constraint as [PCIBIOS_MIN_IO, IO_SPACE_LIMIT] is available to
> > > the PCI controller for I/O tokens...
> >
> > What I am suggesting is to leave PCIBIOS_MIN_IO alone which still
> reserves
> > space for ISA controller and add a PCIBIOS_MIN_DIRECT_IO that will
> reserve
> > space for your direct address I/O on top of PCIBIOS_MIN_IO.
>
> The PCIBIOS_MIN_DIRECT_IO name still suggests having something related
> to
> PCIBIOS_MIN_IO, but it really isn't. We are talking about multiple
> concepts here that are not the same but that are somewhat related:
>
> a) keeping PCI devices from allocating low I/O ports on the PCI bus
> that would conflict with ISA devices behind a bridge of the
> same bus.
>
> b) reserving the low 0x0-0xfff range of the Linux-internal I/O
> space abstraction to a particular LPC or PCI domain to make
> legacy device drivers work that hardcode a particular port
> number.
>
> c) Redirecting inb/outb to call a domain-specific accessor function
> rather than doing the normal MMIO window for an LPC master or
> more generally any arbitrary LPC or PCI domain that has a
> nonstandard I/O space.
> [side note: actually if we generalized this, we could avoid
> assigning an MMIO range for the I/O space on the pci-mvebu
> driver, and that would help free up some other remapping
> windows]
>
> I think there is no need to change a) here, we have PCIBIOS_MIN_IO
> today and even if we don't need it, there is no obvious downside.
> I would also argue that we can ignore b) for the discussion of
> the HiSilicon LPC driver, we just need to assign some range
> of logical addresses to each domain.
>
> That means solving c) is the important problem here, and it
> shouldn't be so hard. We can do this either with a single
> special domain as in the v5 patch series, or by generalizing it
> so that any I/O space mapping gets looked up through the device
> pointer of the bus master.
I am not very on the "generalized" multi-domain solution...
Currently the IO accessors prototypes have an unsigned long addr
as input parameter. If we live in a multi-domain IO system
how can we distinguish inside the accessor which domain addr
belongs to?
Thanks
Gab
>
> Arnd
^ permalink raw reply
* [PATCH v27 1/9] memblock: add memblock_cap_memory_range()
From: Will Deacon @ 2016-11-18 12:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <582DF05A.9050601@arm.com>
On Thu, Nov 17, 2016 at 06:00:58PM +0000, James Morse wrote:
> On 17/11/16 11:19, Will Deacon wrote:
> > It looks much better, thanks! Just one question below.
> >
>
> > On Thu, Nov 17, 2016 at 02:34:24PM +0900, AKASHI Takahiro wrote:
> >> diff --git a/mm/memblock.c b/mm/memblock.c
> >> index 7608bc3..fea1688 100644
> >> --- a/mm/memblock.c
> >> +++ b/mm/memblock.c
> >> @@ -1514,11 +1514,37 @@ void __init memblock_enforce_memory_limit(phys_addr_t limit)
> >> (phys_addr_t)ULLONG_MAX);
> >> }
> >>
> >> +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> >> +{
> >> + int start_rgn, end_rgn;
> >> + int i, ret;
> >> +
> >> + if (!size)
> >> + return;
> >> +
> >> + ret = memblock_isolate_range(&memblock.memory, base, size,
> >> + &start_rgn, &end_rgn);
> >> + if (ret)
> >> + return;
> >> +
> >> + /* remove all the MAP regions */
> >> + for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> >> + if (!memblock_is_nomap(&memblock.memory.regions[i]))
> >> + memblock_remove_region(&memblock.memory, i);
> >
> > In the case that we have only one, giant memblock that covers base all
> > of base + size, can't we end up with start_rgn = end_rgn = 0? In which
>
> Can this happen? If we only have one memblock that exactly spans
> base:(base+size), memblock_isolate_range() will hit the '@rgn is fully
> contained, record it' code and set start_rgn=0,end_rgn=1. (rbase==base,
> rend==end). We only go round the loop once.
>
> If we only have one memblock that is bigger than base:(base+size) we end up with
> three regions, start_rgn=1,end_rgn=2. The trickery here is the '@rgn intersects
> from above' code decreases the loop counter so we process the same entry twice,
> hitting '@rgn is fully contained, record it' the second time round... so we go
> round the loop four times.
>
> I can't see how we hit the:
> > if (rbase >= end)
> > break;
> > if (rend <= base)
> > continue;
>
> code in either case...
I consistently misread that as rend >= end and rbase <= base! In which case,
I agree with your analysis:
Reviewed-by: Will Deacon <will.deacon@arm.com>
The patch could probably still use an ack from an mm person.
Will
^ permalink raw reply
* [RFC 1/6] drm/etnaviv: add binding for the gc320 found in ti socs
From: Russell King - ARM Linux @ 2016-11-18 12:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0bf288ee-33bc-c3b5-389f-08d9bb89ccb5@ti.com>
On Thu, Nov 17, 2016 at 08:53:38PM -0600, Nishanth Menon wrote:
> >diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> >index a6799b0..ce51270 100644
> >--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> >+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> >@@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev)
> > static const struct of_device_id dt_match[] = {
> > { .compatible = "fsl,imx-gpu-subsystem" },
> > { .compatible = "marvell,dove-gpu-subsystem" },
> >+ { .compatible = "ti,gc320-gpu-subsystem" },
We need to get away from this ever-increasing set of compatible
strings here, as this is not long-term maintainable.
What we should have is a common compatible which describes that
the node is compatible with this driver, and then use SoC specific
compatible strings later if we need to (eg, because of some GPU
subsystem SoC specifics.)
So, I'd suggest that we update the documentation and add:
"vivante,gc-gpu-subsystem"
as a common compatible now, and if necessary move on to more specific
compatibles if we need to later.
Also, I'd strongly suggest that no compatibles should contain the ID
number of the GPU core for exactly the same reason - Vivante GPU cores
vary according to features, and we don't want to end up with a long
list of specific compatibles (eg)
"ti,gc2000-and-gc320-and-gc355-gpu-subsystem" because TI
decides to integrate a 3d, 2d and VG core.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU
From: Arnd Bergmann @ 2016-11-18 12:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <62d93a1f-fd34-771b-4280-6b7cde5c61ad@samsung.com>
On Friday, November 18, 2016 8:54:30 AM CET pankaj.dubey wrote:
> >> Please let me know if any concern in this approach.
> >
> > I think ideally we wouldn't even need to know the virtual address
> > outside of smp_scu.c. If we can move all users of the address
> > into that file directly, it could become a local variable and
> > we change scu_power_mode() and scu_get_core_count() instead to
> > not require the address argument.
> >
>
> If we change scu_get_core_count() without address argument, what about
> those platforms which are calling this API very early boot (mostly in
> smp_init_cpus), during this stage we can't use iomap. These platforms
> are doing static mapping of SCU base, and passing virtual address.
>
> Currently following are platforms which needs SCU virtual address at
> very early stage mostly for calling scu_get_core_count(scu_address):
> IMX, ZYNQ, SPEAr, and OMAP2.
Ah, you are right, I missed how this is done earlier than the
scu_enable() call.
> I can think of handling of these platform's early need of SCU in the
> following way:
>
> Probably we need something like early_a9_scu_get_core_count() which can
> be handled in this way in smp_scu.c file itself:
>
> +static struct map_desc scu_map __initdata = {
> + .length = SZ_256,
> + .type = MT_DEVICE,
> +};
> +
> +static void __iomem *early_scu_map_io(void)
> +{
> + unsigned long base;
> + void __iomem *scu_base;
> +
> + base = scu_a9_get_base();
> + scu_map.pfn = __phys_to_pfn(base);
> + scu_map.virtual = base;
> + iotable_init(&scu_map, 1);
> + scu_base = (void __iomem *)base;
> + return scu_base;
> +}
Unfortunately, this doesn't work because scu_map.virtual must be
picked by the platform code in a way that doesn't conflict
with anything else. Setting up the iotable is probably not
something we should move into the smp_scu.c file but leave where
it currently is. You copied the trick from zynq that happens
to work there but probably not on the other three.
At some point we decided that at least new platforms should
always get the core count from DT rather than from the SCU,
but I don't know if we have to keep supporting old DTB files
without a full description of the CPUs on any of the
four platforms.
I see that there are four other users of scu_get_core_count()
that all call it from ->prepare_cpus, and we can probably
just use num_possible_cpus() at that point as the count
must have been initialized from DT already.
> +/*
> + * early_a9_scu_get_core_count - Get number of CPU cores at very early boot
> + * Only platforms which needs number_of_cores during early boot should
> call this
> + */
> +unsigned int __init early_a9_scu_get_core_count(void)
> +{
> + void __iomem *scu_base;
> +
> + scu_base = early_scu_map_io();
> + return scu_get_core_count(scu_base);
> +}
> +
>
> Please let me know how about using above approach to simplify platform
> specific code of early users of SCU address.
>
> Also for rest platforms, which are not using scu base at very early
> stage, but are using virtual address which is mapped either from SCU
> device node or using s9_scu_get_base() to map to SCU virtual address. To
> separate these two we discussed to separate scu_enable in two helper
> APIs as of_scu_enable and s9_scu_enable. So if we change
> scu_get_core_count which do not requires address argument, this also
> needs to be separated in two as of_scu_get_core_count and
> s9_scu_get_core_count.
We can probably leave get_core_count code alone for now, or
we change it so that we pass a virtual address into it
from the platform and have the SCU mapped there. One nice
property of the early mapping is that the later ioremap()
will just return the same virtual address, so we don't get
a double mapping when calling the scu_enable variant later.
Adding two functions of each doesn't sound too great though,
it would make the API more complicated when the intention was
to make it simpler by leaving out the address argument.
Maybe something like this?
diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c
index 72f9241ad5db..c248a16e980f 100644
--- a/arch/arm/kernel/smp_scu.c
+++ b/arch/arm/kernel/smp_scu.c
@@ -24,6 +24,8 @@
#define SCU_INVALIDATE 0x0c
#define SCU_FPGA_REVISION 0x10
+static void __iomem *scu_base_addr;
+
#ifdef CONFIG_SMP
/*
* Get the number of CPU cores from the SCU configuration
@@ -41,6 +43,9 @@ void scu_enable(void __iomem *scu_base)
{
u32 scu_ctrl;
+ if (scu_base)
+ scu_base = scu_base_addr;
+
#ifdef CONFIG_ARM_ERRATA_764369
/* Cortex-A9 only */
if ((read_cpuid_id() & 0xff0ffff0) == 0x410fc090) {
@@ -85,6 +90,9 @@ int scu_power_mode(void __iomem *scu_base, unsigned int mode)
unsigned int val;
int cpu = MPIDR_AFFINITY_LEVEL(cpu_logical_map(smp_processor_id()), 0);
+ if (scu_base)
+ scu_base = scu_base_addr;
+
if (mode > 3 || mode == 1 || cpu > 3)
return -EINVAL;
@@ -94,3 +102,31 @@ int scu_power_mode(void __iomem *scu_base, unsigned int mode)
return 0;
}
+
+int __init scu_probe_a9(void)
+{
+ phys_addr_t base;
+
+ if (!scu_a9_has_base)
+ return -ENODEV;
+
+ base = scu_a9_get_base()
+ if (!base)
+ return -ENODEV;
+
+ scu_base_addr = ioremap(base, PAGE_SIZE);
+ if (scu_base_addr)
+ return -ENOMEM;
+
+ return scu_get_core_count(scu_base_addr);
+}
+
+int __init scu_probe_dt(void)
+{
+ ...
+ scu_base_addr = of_iomap(np, 0);
+ if (scu_base_addr)
+ return -ENOMEM;
+
+ return scu_get_core_count(scu_base_addr);
+}
Arnd
^ permalink raw reply related
* [arm-soc:qcom/arm64 13/13] arch/arm64/boot/dts/qcom/msm8994.dtsi:14:48: fatal error: dt-bindings/clock/qcom,gcc-msm8994.h: No such file or directory
From: kbuild test robot @ 2016-11-18 12:20 UTC (permalink / raw)
To: linux-arm-kernel
tree: https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git qcom/arm64
head: feeaf56ac78d283efe65ea60ec999d4bf3cf395e
commit: feeaf56ac78d283efe65ea60ec999d4bf3cf395e [13/13] arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout feeaf56ac78d283efe65ea60ec999d4bf3cf395e
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
In file included from arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts:16:0:
>> arch/arm64/boot/dts/qcom/msm8994.dtsi:14:48: fatal error: dt-bindings/clock/qcom,gcc-msm8994.h: No such file or directory
#include <dt-bindings/clock/qcom,gcc-msm8994.h>
^
compilation terminated.
vim +14 arch/arm64/boot/dts/qcom/msm8994.dtsi
8 * but WITHOUT ANY WARRANTY; without even the implied warranty of
9 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
10 * GNU General Public License for more details.
11 */
12
13 #include <dt-bindings/interrupt-controller/arm-gic.h>
> 14 #include <dt-bindings/clock/qcom,gcc-msm8994.h>
15
16 / {
17 model = "Qualcomm Technologies, Inc. MSM 8994";
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 31923 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161118/0ec2fd4b/attachment-0001.gz>
^ permalink raw reply
* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-11-18 12:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <EE11001F9E5DDD47B7634E2F8A612F2E1F921146@lhreml507-mbx>
On Friday, November 18, 2016 12:07:28 PM CET Gabriele Paoloni wrote:
> > From: Arnd Bergmann [mailto:arnd at arndb.de]
> > On Monday, November 14, 2016 11:26:25 AM CET liviu.dudau at arm.com wrote:
> > > On Mon, Nov 14, 2016 at 08:26:42AM +0000, Gabriele Paoloni wrote:
> > > > > Nope, that is not what it means. It means that PCI devices can
> > see I/O
> > > > > addresses
> > > > > on the bus that start from 0. There never was any usage for non-
> > PCI
> > > > > controllers
> > > >
> > > > So I am a bit confused...
> > > > From http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> > > > It seems that ISA buses operate on cpu I/O address range [0,
> > 0xFFF].
> > > > I thought that was the reason why for most architectures we have
> > > > PCIBIOS_MIN_IO equal to 0x1000 (so I thought that ISA controllers
> > > > usually use [0, PCIBIOS_MIN_IO - 1] )
> > >
> > > First of all, cpu I/O addresses is an x86-ism. ARM architectures and
> > others
> > > have no separate address space for I/O, it is all merged into one
> > unified
> > > address space. So, on arm/arm64 for example, PCIBIOS_MIN_IO = 0 could
> > mean
> > > that we don't care about ISA I/O because the platform does not
> > support having
> > > an ISA bus (e.g.).
> >
> > I think to be more specific, PCIBIOS_MIN_IO=0 would indicate that you
> > cannot
> > have a PCI-to-ISA or PCI-to-LPC bridge in any PCI domain. This is
> > different
> > from having an LPC master outside of PCI, as that lives in its own
> > domain
> > and has a separately addressable I/O space.
>
> Yes correct so if we go for the single domain solution arch that
> have PCIBIOS_MIN_IO=0 cannot support special devices such as LPC
> unless we also redefine PCIBIOS_MIN_IO, right?
This is what I was referring to below as the difference between
a) and b): Setting PCIBIOS_MIN_IO=0 means you cannot have LPC
behind PCI, but it shouldn't stop you from having a separate
LPC bridge.
> > The PCIBIOS_MIN_DIRECT_IO name still suggests having something related
> > to
> > PCIBIOS_MIN_IO, but it really isn't. We are talking about multiple
> > concepts here that are not the same but that are somewhat related:
> >
> > a) keeping PCI devices from allocating low I/O ports on the PCI bus
> > that would conflict with ISA devices behind a bridge of the
> > same bus.
> >
> > b) reserving the low 0x0-0xfff range of the Linux-internal I/O
> > space abstraction to a particular LPC or PCI domain to make
> > legacy device drivers work that hardcode a particular port
> > number.
> >
> > c) Redirecting inb/outb to call a domain-specific accessor function
> > rather than doing the normal MMIO window for an LPC master or
> > more generally any arbitrary LPC or PCI domain that has a
> > nonstandard I/O space.
> > [side note: actually if we generalized this, we could avoid
> > assigning an MMIO range for the I/O space on the pci-mvebu
> > driver, and that would help free up some other remapping
> > windows]
> >
> > I think there is no need to change a) here, we have PCIBIOS_MIN_IO
> > today and even if we don't need it, there is no obvious downside.
> > I would also argue that we can ignore b) for the discussion of
> > the HiSilicon LPC driver, we just need to assign some range
> > of logical addresses to each domain.
> >
> > That means solving c) is the important problem here, and it
> > shouldn't be so hard. We can do this either with a single
> > special domain as in the v5 patch series, or by generalizing it
> > so that any I/O space mapping gets looked up through the device
> > pointer of the bus master.
>
> I am not very on the "generalized" multi-domain solution...
> Currently the IO accessors prototypes have an unsigned long addr
> as input parameter. If we live in a multi-domain IO system
> how can we distinguish inside the accessor which domain addr
> belongs to?
The easiest change compared to the v5 code would be to walk
a linked list of 'struct extio_ops' structures rather than
assuming there is only ever one of them. I think one of the
earlier versions actually did this.
Another option the IA64 approach mentioned in another subthread
today, looking up the operations based on an index from the
upper bits of the port number. If we do this, we probably
want to do that for all PIO access and replace the entire
virtual address remapping logic with that. I think Bjorn
in the past argued in favor of such an approach, while I
advocated the current scheme for simplicity based on how
every I/O space these days is just memory mapped (which now
turned out to be false, both on powerpc and arm64).
Arnd
^ permalink raw reply
* [PATCH] arm: spin one more cycle in timer-based delays
From: Mason @ 2016-11-18 12:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118120630.GJ13470@arm.com>
On 18/11/2016 13:06, Will Deacon wrote:
> On Tue, Nov 15, 2016 at 02:36:33PM +0100, Mason wrote:
>
>> When polling a tick counter in a busy loop, one might sample the
>> counter just *before* it is updated, and then again just *after*
>> it is updated. In that case, while mathematically v2-v1 equals 1,
>> only epsilon has really passed.
>>
>> So, if a caller requests an N-cycle delay, we spin until v2-v1
>> is strictly greater than N to avoid these random corner cases.
>>
>> Signed-off-by: Mason <slash.tmp@free.fr>
>> ---
>> arch/arm/lib/delay.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c
>> index 69aad80a3af4..3f1cd15ca102 100644
>> --- a/arch/arm/lib/delay.c
>> +++ b/arch/arm/lib/delay.c
>> @@ -60,7 +60,7 @@ static void __timer_delay(unsigned long cycles)
>> {
>> cycles_t start = get_cycles();
>>
>> - while ((get_cycles() - start) < cycles)
>> + while ((get_cycles() - start) <= cycles)
>> cpu_relax();
>> }
>
> I thought a bit about this last night. It is well known that the delay
> routines are not guaranteed to be accurate, and I don't *think* that's
> limited to over-spinning, so arguably this isn't a bug. However, taking
> the approach that "drivers should figure it out" is also unhelpful,
> because the frequency of the underlying counter isn't generally known
> and so drivers can't simply adjust the delay parameter.
>
> If you want to go ahead with this change, I do think that you should fix
> the other architectures for consistency (particularly arm64, which is
> likely to share drivers with arch/arm/). However, given that I don't
> think you've seen a failure in practice, it might be a hard sell to get
> the patches picked up, especially given the deliberately vague guarantees
> offered by the delay loop.
Hello Will,
Thanks for the in-depth analysis.
This is, conceptually, the first patch in a 2-patch series, and perhaps
the intent would have been clearer had I posted the series, along with
full rationale in the cover letter. I'll do that next week, so everyone
can judge with all the information in hand.
Regards.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox