Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] irqchip/gicv3-its: Enable cacheable attribute Read-allocate hints
From: Shanker Donthineni @ 2016-11-08  7:18 UTC (permalink / raw)
  To: linux-arm-kernel

Read-allocation hints are not enabled for both the GIC-ITS and GICR
tables. This forces the hardware to always read the table contents
from an external memory (DDR) which is slow compared to cache memory.
Most of the tables are often read by hardware. So, it's better to
enable Read-allocate hints in addition to Write-allocate hints in
order to improve the GICR_PEND, GICR_PROP, Collection, Device, and
vCPU tables lookup time.

Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
---
Implemented a test case to prove that enabling Read Allocation hints
improves ITS lookup time ~15% while delivering a LPI event. Used the
ITS command INV to analyze time spent in device, collection, prop and
pending table lookups.
  
Pseudo code:
    Create a fake ITS device.
    Record PMU cycle counter before sending INV command.
    Build and send ITS INT command.
         ITS hardware triggers device table lookup.
              ITTE table & collection table lookup.
          ITS property table lookup.
          ITS pending table lookup.
          Deliver interrupt to CPU interface.
    do_IRQ() called.
    Measure the total CPU cycle spent to reach this point. 

Without ReadAllocation hints:
/sys/kernel/debug # echo 100 > lpitest
[   94.693968] CPU[1] niter=100 cycles=0x8dfc0 avg=0x16b7 min=0x1652

With ReadAllocation hints:
/sys/kernel/debug # echo 100 > lpitest
[   98.617873] CPU[1] niter=100 cycles=0x7df49 avg=0x1427 min=0x1388

 drivers/irqchip/irq-gic-v3-its.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index c5dee30..227a1eb 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -961,7 +961,7 @@ static bool its_parse_baser_device(struct its_node *its, struct its_baser *baser
 				   u32 psz, u32 *order)
 {
 	u64 esz = GITS_BASER_ENTRY_SIZE(its_read_baser(its, baser));
-	u64 val = GITS_BASER_InnerShareable | GITS_BASER_WaWb;
+	u64 val = GITS_BASER_InnerShareable | GITS_BASER_RaWaWb;
 	u32 ids = its->device_ids;
 	u32 new_order = *order;
 	bool indirect = false;
@@ -1026,7 +1026,7 @@ static int its_alloc_tables(struct its_node *its)
 	u64 typer = gic_read_typer(its->base + GITS_TYPER);
 	u32 ids = GITS_TYPER_DEVBITS(typer);
 	u64 shr = GITS_BASER_InnerShareable;
-	u64 cache = GITS_BASER_WaWb;
+	u64 cache = GITS_BASER_RaWaWb;
 	u32 psz = SZ_64K;
 	int err, i;
 
@@ -1123,7 +1123,7 @@ static void its_cpu_init_lpis(void)
 	/* set PROPBASE */
 	val = (page_to_phys(gic_rdists->prop_page) |
 	       GICR_PROPBASER_InnerShareable |
-	       GICR_PROPBASER_WaWb |
+	       GICR_PROPBASER_RaWaWb |
 	       ((LPI_NRBITS - 1) & GICR_PROPBASER_IDBITS_MASK));
 
 	writeq_relaxed(val, rbase + GICR_PROPBASER);
@@ -1148,7 +1148,7 @@ static void its_cpu_init_lpis(void)
 	/* set PENDBASE */
 	val = (page_to_phys(pend_page) |
 	       GICR_PENDBASER_InnerShareable |
-	       GICR_PENDBASER_WaWb);
+	       GICR_PENDBASER_RaWaWb);
 
 	writeq_relaxed(val, rbase + GICR_PENDBASER);
 	tmp = readq_relaxed(rbase + GICR_PENDBASER);
@@ -1712,7 +1712,7 @@ static int __init its_probe_one(struct resource *res,
 		goto out_free_tables;
 
 	baser = (virt_to_phys(its->cmd_base)	|
-		 GITS_CBASER_WaWb		|
+		 GITS_CBASER_RaWaWb		|
 		 GITS_CBASER_InnerShareable	|
 		 (ITS_CMD_QUEUE_SZ / SZ_4K - 1)	|
 		 GITS_CBASER_VALID);
-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply related

* [PATCH v2] irqchip/gicv3-its: Enable cacheable attribute Read-allocate hints
From: Shanker Donthineni @ 2016-11-08  7:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478589498-17696-1-git-send-email-shankerd@codeaurora.org>

Hi Marc,

I attached the test code that I used to benchmark ReadAllocate hints 
performance on Qualcomm hardware.

Shanker


On 11/08/2016 01:18 AM, Shanker Donthineni wrote:
> Read-allocation hints are not enabled for both the GIC-ITS and GICR
> tables. This forces the hardware to always read the table contents
> from an external memory (DDR) which is slow compared to cache memory.
> Most of the tables are often read by hardware. So, it's better to
> enable Read-allocate hints in addition to Write-allocate hints in
> order to improve the GICR_PEND, GICR_PROP, Collection, Device, and
> vCPU tables lookup time.
>
> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
> ---
> Implemented a test case to prove that enabling Read Allocation hints
> improves ITS lookup time ~15% while delivering a LPI event. Used the
> ITS command INV to analyze time spent in device, collection, prop and
> pending table lookups.
>    
> Pseudo code:
>      Create a fake ITS device.
>      Record PMU cycle counter before sending INV command.
>      Build and send ITS INT command.
>           ITS hardware triggers device table lookup.
>                ITTE table & collection table lookup.
>            ITS property table lookup.
>            ITS pending table lookup.
>            Deliver interrupt to CPU interface.
>      do_IRQ() called.
>      Measure the total CPU cycle spent to reach this point.
>
> Without ReadAllocation hints:
> /sys/kernel/debug # echo 100 > lpitest
> [   94.693968] CPU[1] niter=100 cycles=0x8dfc0 avg=0x16b7 min=0x1652
>
> With ReadAllocation hints:
> /sys/kernel/debug # echo 100 > lpitest
> [   98.617873] CPU[1] niter=100 cycles=0x7df49 avg=0x1427 min=0x1388
>
>   drivers/irqchip/irq-gic-v3-its.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index c5dee30..227a1eb 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -961,7 +961,7 @@ static bool its_parse_baser_device(struct its_node *its, struct its_baser *baser
>   				   u32 psz, u32 *order)
>   {
>   	u64 esz = GITS_BASER_ENTRY_SIZE(its_read_baser(its, baser));
> -	u64 val = GITS_BASER_InnerShareable | GITS_BASER_WaWb;
> +	u64 val = GITS_BASER_InnerShareable | GITS_BASER_RaWaWb;
>   	u32 ids = its->device_ids;
>   	u32 new_order = *order;
>   	bool indirect = false;
> @@ -1026,7 +1026,7 @@ static int its_alloc_tables(struct its_node *its)
>   	u64 typer = gic_read_typer(its->base + GITS_TYPER);
>   	u32 ids = GITS_TYPER_DEVBITS(typer);
>   	u64 shr = GITS_BASER_InnerShareable;
> -	u64 cache = GITS_BASER_WaWb;
> +	u64 cache = GITS_BASER_RaWaWb;
>   	u32 psz = SZ_64K;
>   	int err, i;
>   
> @@ -1123,7 +1123,7 @@ static void its_cpu_init_lpis(void)
>   	/* set PROPBASE */
>   	val = (page_to_phys(gic_rdists->prop_page) |
>   	       GICR_PROPBASER_InnerShareable |
> -	       GICR_PROPBASER_WaWb |
> +	       GICR_PROPBASER_RaWaWb |
>   	       ((LPI_NRBITS - 1) & GICR_PROPBASER_IDBITS_MASK));
>   
>   	writeq_relaxed(val, rbase + GICR_PROPBASER);
> @@ -1148,7 +1148,7 @@ static void its_cpu_init_lpis(void)
>   	/* set PENDBASE */
>   	val = (page_to_phys(pend_page) |
>   	       GICR_PENDBASER_InnerShareable |
> -	       GICR_PENDBASER_WaWb);
> +	       GICR_PENDBASER_RaWaWb);
>   
>   	writeq_relaxed(val, rbase + GICR_PENDBASER);
>   	tmp = readq_relaxed(rbase + GICR_PENDBASER);
> @@ -1712,7 +1712,7 @@ static int __init its_probe_one(struct resource *res,
>   		goto out_free_tables;
>   
>   	baser = (virt_to_phys(its->cmd_base)	|
> -		 GITS_CBASER_WaWb		|
> +		 GITS_CBASER_RaWaWb		|
>   		 GITS_CBASER_InnerShareable	|
>   		 (ITS_CMD_QUEUE_SZ / SZ_4K - 1)	|
>   		 GITS_CBASER_VALID);

-- 
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-irqchip-gicv3-its-Test-code-for-measuring-Read-alloc.patch
Type: text/x-patch
Size: 9551 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161108/e3368df4/attachment-0001.bin>

^ permalink raw reply

* [PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver
From: Anurup M @ 2016-11-08  7:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <58217871.6050609@huawei.com>


On Tuesday 08 November 2016 12:32 PM, Tan Xiaojun wrote:
> On 2016/11/7 21:26, Arnd Bergmann wrote:
>> On Wednesday, November 2, 2016 11:42:46 AM CET Anurup M wrote:
>>> From: Tan Xiaojun <tanxiaojun@huawei.com>
>>>
>>> 	The Hisilicon Djtag is an independent component which connects
>>> 	with some other components in the SoC by Debug Bus. This driver
>>> 	can be configured to access the registers of connecting components
>>> 	(like L3 cache) during real time debugging.
>> The formatting of the text seems odd, please remove the leading spaces.
>>
> Sorry for that. We will fix it.
>
>>>   drivers/soc/Kconfig                 |   1 +
>>>   drivers/soc/Makefile                |   1 +
>>>   drivers/soc/hisilicon/Kconfig       |  12 +
>>>   drivers/soc/hisilicon/Makefile      |   1 +
>>>   drivers/soc/hisilicon/djtag.c       | 639 ++++++++++++++++++++++++++++++++++++
>>>   include/linux/soc/hisilicon/djtag.h |  38 +++
>> Do you expect other drivers to be added that reference this interface?
>> If not, or if you are unsure, just put all of it under drivers/perf
>> so we don't introduce a global API that has only one user.
>>
> OK. For now, this suggestion sounds good.
>
>>> +
>>> +#include <linux/bitops.h>
>>> +#include <linux/init.h>
>>> +#include <linux/list.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/spinlock.h>
>>> +
>>> +#include <asm-generic/delay.h>
>> Never include files from asm-generic directly except from
>> an architecture specific asm/*.h header file.
>>
>>
> OK. Sorry for that.
>
>>> +DEFINE_IDR(djtag_hosts_idr);
>> make this static
>>
> OK.
>
>>> +static void djtag_read32_relaxed(void __iomem *regs_base, u32 off, u32 *value)
>>> +{
>>> +	void __iomem *reg_addr = regs_base + off;
>>> +
>>> +	*value = readl_relaxed(reg_addr);
>>> +}
>>> +
>>> +static void djtag_write32(void __iomem *regs_base, u32 off, u32 val)
>>> +{
>>> +	void __iomem *reg_addr = regs_base + off;
>>> +
>>> +	writel(val, reg_addr);
>>> +}
>> This looks like an odd combination of interfaces.
>> Why can the reads be "relaxed" when the writes can not?
>>
>> Generally speaking, I'd advise to always use non-relaxed accessors
>> unless there is a strong performance reason, and in that case there
>> should be a comment explaining the use at each of the callers
>> of a relaxed accessor.
>>
> Yes, it is our mistake.
Shall use non-relaxed version and make it consistent.
>>> +	/* ensure the djtag operation is done */
>>> +	do {
>>> +		djtag_read32_relaxed(regs_base, SC_DJTAG_MSTR_START_EN_EX, &rd);
>>> +
>>> +		if (!(rd & DJTAG_MSTR_START_EN_EX))
>>> +			break;
>>> +
>>> +		udelay(1);
>>> +	} while (timeout--);
>> This one is obviously not performance critical at all, so use a non-relaxed
>> accessor. Same for the other two in this function.
>>
>> Are these functions ever called from atomic context? If yes, please document
>> from what context they can be called, otherwise please consider changing
>> the udelay calls into sleeping waits.
>>
> Yes, this is not reentrant.
The read/write functions shall also be called from irq handler (for 
handling counter overflow).
So need to use udelay calls. Shall Document it in v2.
>>> +int hisi_djtag_writel(struct hisi_djtag_client *client, u32 offset, u32 mod_sel,
>>> +							u32 mod_mask, u32 val)
>>> +{
>>> +	void __iomem *reg_map = client->host->sysctl_reg_map;
>>> +	unsigned long flags;
>>> +	int ret = 0;
>>> +
>>> +	spin_lock_irqsave(&client->host->lock, flags);
>>> +	ret = client->host->djtag_readwrite(reg_map, offset, mod_sel, mod_mask,
>>> +					true, val, 0, NULL);
>>> +	if (ret)
>>> +		pr_err("djtag_writel: error! ret=%d\n", ret);
>>> +	spin_unlock_irqrestore(&client->host->lock, flags);
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(hisi_djtag_writel);
>> That would of course imply changing the spinlock to a mutex here as well.
>>
>>> +static const struct of_device_id djtag_of_match[] = {
>>> +	/* for hip05(D02) cpu die */
>>> +	{ .compatible = "hisilicon,hip05-cpu-djtag-v1",
>>> +		.data = (void *)djtag_readwrite_v1 },
>>> +	/* for hip05(D02) io die */
>>> +	{ .compatible = "hisilicon,hip05-io-djtag-v1",
>>> +		.data = (void *)djtag_readwrite_v1 },
>>> +	/* for hip06(D03) cpu die */
>>> +	{ .compatible = "hisilicon,hip06-cpu-djtag-v1",
>>> +		.data = (void *)djtag_readwrite_v1 },
>>> +	/* for hip06(D03) io die */
>>> +	{ .compatible = "hisilicon,hip06-io-djtag-v2",
>>> +		.data = (void *)djtag_readwrite_v2 },
>>> +	/* for hip07(D05) cpu die */
>>> +	{ .compatible = "hisilicon,hip07-cpu-djtag-v2",
>>> +		.data = (void *)djtag_readwrite_v2 },
>>> +	/* for hip07(D05) io die */
>>> +	{ .compatible = "hisilicon,hip07-io-djtag-v2",
>>> +		.data = (void *)djtag_readwrite_v2 },
>>> +	{},
>>> +};
>> If these are backwards compatible, just mark them as compatible in DT,
>> e.g. hip06 can use
>>
>> 	compatible = "hisilicon,hip06-cpu-djtag-v1", "hisilicon,hip05-cpu-djtag-v1";
>>
>> so you can tell the difference if you need to, but the driver only has to
>> list the oldest one here.
>>
>> What is the difference between the cpu and io djtag interfaces?
On some chips like hip06, the djtag version is different for IO die.

Thanks,
Anurup
>>
>> I think you can also drop the '(void *)'.
>>
> OK. We will consider it.
>
> Thanks.
> Xiaojun.
>
>>> +static void djtag_register_devices(struct hisi_djtag_host *host)
>>> +{
>>> +	struct device_node *node;
>>> +	struct hisi_djtag_client *client;
>>> +
>>> +	if (!host->of_node)
>>> +		return;
>>> +
>>> +	for_each_available_child_of_node(host->of_node, node) {
>>> +		if (of_node_test_and_set_flag(node, OF_POPULATED))
>>> +			continue;
>>> +		client = hisi_djtag_of_register_device(host, node);
>>> +		list_add(&client->next, &host->client_list);
>>> +	}
>>> +}
>> Can you explain your thoughts behind creating a new bus type
>> and adding the child devices manually rather than using
>> platform_device structures with of_platform_populate()?
>>
>> Do you expect to see other implementations of this bus type
>> with incompatible bus drivers?
>>
>> 	Arnd
>>
>>
>> .
>>
>

^ permalink raw reply

* [PATCH 3/3] ARM: gr8: evb: Add i2s codec
From: Maxime Ripard @ 2016-11-08  7:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAGb2v65bzWJpEnmwk89Qkn+_QawcPtpCJdA+90Dgc68FLRNQqw@mail.gmail.com>

On Mon, Nov 07, 2016 at 10:11:45PM +0800, Chen-Yu Tsai wrote:
> On Mon, Nov 7, 2016 at 9:08 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > The GR8-EVB comes with a wm8978 codec connected to the i2s bus.
> >
> > Add a card in order to have it working
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  arch/arm/boot/dts/ntc-gr8-evb.dts | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/ntc-gr8-evb.dts b/arch/arm/boot/dts/ntc-gr8-evb.dts
> > index 12b4317a4383..5291e425caf9 100644
> > --- a/arch/arm/boot/dts/ntc-gr8-evb.dts
> > +++ b/arch/arm/boot/dts/ntc-gr8-evb.dts
> > @@ -76,6 +76,20 @@
> >                 default-brightness-level = <8>;
> >         };
> >
> > +       i2s {
> 
> "sound" might be a better node name? The I2S controllers are also
> named "i2s".

I know, but we also had the codec and SPDIF on this board, so sound
was too generic to be meaningful I guess. I don't really care about
the name though, if you have any suggestion...

> Otherwise,
> 
> Acked-by: Chen-Yu Tsai <wens@csie.org>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161108/9cd82ae9/attachment.sig>

^ permalink raw reply

* [PATCH 3/3] ARM: gr8: evb: Add i2s codec
From: Chen-Yu Tsai @ 2016-11-08  7:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161108074414.vvnjj7aa26sloi2f@lukather>

On Tue, Nov 8, 2016 at 3:44 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Mon, Nov 07, 2016 at 10:11:45PM +0800, Chen-Yu Tsai wrote:
>> On Mon, Nov 7, 2016 at 9:08 PM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > The GR8-EVB comes with a wm8978 codec connected to the i2s bus.
>> >
>> > Add a card in order to have it working
>> >
>> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> > ---
>> >  arch/arm/boot/dts/ntc-gr8-evb.dts | 14 ++++++++++++++
>> >  1 file changed, 14 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/arm/boot/dts/ntc-gr8-evb.dts b/arch/arm/boot/dts/ntc-gr8-evb.dts
>> > index 12b4317a4383..5291e425caf9 100644
>> > --- a/arch/arm/boot/dts/ntc-gr8-evb.dts
>> > +++ b/arch/arm/boot/dts/ntc-gr8-evb.dts
>> > @@ -76,6 +76,20 @@
>> >                 default-brightness-level = <8>;
>> >         };
>> >
>> > +       i2s {
>>
>> "sound" might be a better node name? The I2S controllers are also
>> named "i2s".
>
> I know, but we also had the codec and SPDIF on this board, so sound
> was too generic to be meaningful I guess. I don't really care about
> the name though, if you have any suggestion...

Well people seem to use "sound" for the sound card nodes...

What about "sound-analog" for this one, and "sound-spdif" for the
SPDIF simple card? Or "analog-sound" and "spdif-sound" if that looks
better.


ChenYu

>> Otherwise,
>>
>> Acked-by: Chen-Yu Tsai <wens@csie.org>
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

^ permalink raw reply

* [PATCH 1/6] clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards
From: Gabriel Fernandez @ 2016-11-08  8:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAFvLkMQxvo=2Ak033n2XkPgdDfU5DLCMF7dj487QFB0e7WceCQ@mail.gmail.com>

Hi Rados?aw

Many thanks for reviewing.

On 11/07/2016 03:57 PM, Rados?aw Pietrzyk wrote:
>> +static struct clk_hw *clk_register_pll_div(const char *name,
>> +               const char *parent_name, unsigned long flags,
>> +               void __iomem *reg, u8 shift, u8 width,
>> +               u8 clk_divider_flags, const struct clk_div_table *table,
>> +               struct clk_hw *pll_hw, spinlock_t *lock)
>> +{
>> +       struct stm32f4_pll_div *pll_div;
>> +       struct clk_hw *hw;
>> +       struct clk_init_data init;
>> +       int ret;
>> +
>> +       /* allocate the divider */
>> +       pll_div = kzalloc(sizeof(*pll_div), GFP_KERNEL);
>> +       if (!pll_div)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       init.name = name;
>> +       init.ops = &stm32f4_pll_div_ops;
>> +       init.flags = flags;
> Maybe it's worth to have CLK_SET_RATE_PARENT here and the VCO clock
> should have CLK_SET_RATE_GATE flag and we can get rid of custom
> divider ops.
I don't want to offer the possibility to change the vco clock through 
the divisor of the pll (only by a boot-loader or by DT).

e.g. if i make a set rate on lcd-tft clock, i don't want to change the 
SAI frequencies.

I used same structure for internal divisors of the pll (p, q, r) and for 
post divisors (plli2s-q-div, pllsai-q-div & pllsai-r-div).
That why the CLK_SET_RATE_PARENT flag is transmit by parameter.

These divisors are similar because we have to switch off the pll before 
changing the rate.

>
>
>> -static void stm32f4_rcc_register_pll(const char *hse_clk, const char *hsi_clk)
>> +
>> +static struct clk_hw *stm32f4_rcc_register_pll(const char *pllsrc,
>> +               const struct stm32f4_pll_data *data,  spinlock_t *lock)
>>   {
>> -       unsigned long pllcfgr = readl(base + STM32F4_RCC_PLLCFGR);
>> +       struct stm32f4_pll *pll;
>> +       struct clk_init_data init = { NULL };
>> +       void __iomem *reg;
>> +       struct clk_hw *pll_hw;
>> +       int ret;
>> +
>> +       pll = kzalloc(sizeof(*pll), GFP_KERNEL);
>> +       if (!pll)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       init.name = data->vco_name;
>> +       init.ops = &stm32f4_pll_gate_ops;
>> +       init.flags = CLK_IGNORE_UNUSED;
> CLK_SET_RATE_GATE here
>
> Moreover why not having VCO as a composite clock from gate and mult ?
Yes, that sounds a good idea.

> According to docs SAI VCO (don't know about I2S ) must be within
> certain range so clk_set_rate_range should be somewhere.

^ permalink raw reply

* [PATCH 1/6] clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards
From: Radosław Pietrzyk @ 2016-11-08  8:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <ebadfacf-73fb-01a6-791a-324daad5e695@st.com>

2016-11-08 9:35 GMT+01:00 Gabriel Fernandez <gabriel.fernandez@st.com>:
> Hi Rados?aw
>
> Many thanks for reviewing.
>
> On 11/07/2016 03:57 PM, Rados?aw Pietrzyk wrote:
>>>
>>> +static struct clk_hw *clk_register_pll_div(const char *name,
>>> +               const char *parent_name, unsigned long flags,
>>> +               void __iomem *reg, u8 shift, u8 width,
>>> +               u8 clk_divider_flags, const struct clk_div_table *table,
>>> +               struct clk_hw *pll_hw, spinlock_t *lock)
>>> +{
>>> +       struct stm32f4_pll_div *pll_div;
>>> +       struct clk_hw *hw;
>>> +       struct clk_init_data init;
>>> +       int ret;
>>> +
>>> +       /* allocate the divider */
>>> +       pll_div = kzalloc(sizeof(*pll_div), GFP_KERNEL);
>>> +       if (!pll_div)
>>> +               return ERR_PTR(-ENOMEM);
>>> +
>>> +       init.name = name;
>>> +       init.ops = &stm32f4_pll_div_ops;
>>> +       init.flags = flags;
>>
>> Maybe it's worth to have CLK_SET_RATE_PARENT here and the VCO clock
>> should have CLK_SET_RATE_GATE flag and we can get rid of custom
>> divider ops.
>
> I don't want to offer the possibility to change the vco clock through the
> divisor of the pll (only by a boot-loader or by DT).
>
> e.g. if i make a set rate on lcd-tft clock, i don't want to change the SAI
> frequencies.
>
> I used same structure for internal divisors of the pll (p, q, r) and for
> post divisors (plli2s-q-div, pllsai-q-div & pllsai-r-div).
> That why the CLK_SET_RATE_PARENT flag is transmit by parameter.
>
> These divisors are similar because we have to switch off the pll before
> changing the rate.
>
But changing pll and lcd dividers only may not be enough for getting
very specific pixelclocks and that might require changing the VCO
frequency itself. The rest of the SAI tree should be recalculated
then.

^ permalink raw reply

* [PATCH] ARM: tegra: nyan: Mark all USB ports as host
From: Peter De Schrijver @ 2016-11-08  8:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <13e7ff98-8bc2-ccf7-94bb-4e1d3c61b20d@nvidia.com>

On Mon, Nov 07, 2016 at 02:09:31PM +0000, Jon Hunter wrote:
> 
> On 07/11/16 13:28, Thierry Reding wrote:
> > * PGP Signed by an unknown key
> > 
> > On Sun, Sep 18, 2016 at 12:28:52PM +0200, Paul Kocialkowski wrote:
> >> Nyan boards only have host USB ports (2 external, 1 internal), there is
> >> no OTG-enabled connector.
> >>
> >> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> >> ---
> >>  arch/arm/boot/dts/tegra124-nyan.dtsi | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Where is this information coming from? I don't have one of the Nyans
> > myself, but one of the Tegra132 devices I have, which I think was
> > derived from one of the Nyans uses one of the external host ports as
> > forced recovery port, for which it would need OTG.
> > 
> > I suspect that the way to get U-Boot onto the Nyans is via tegrarcm?
> > In that case I think one of the ports must be OTG.
> 
> It is true that the port on the back on the nyan-big can be used with
> recovery mode. I was thinking that this is not a true OTG port as it is
> just a 4-pin type A socket and does not have an ID pin. Thinking some
> more about this the USB spec does include a "Host Negotiation Protocol
> (HNP)" that allows a host and device to swap roles and so keeping it as
> OTG seems valid afterall.

I don't think the bootrom implements that though. I expect recovery mode
to just program the controller in device mode, without performing any
negotiation.

Peter.

^ permalink raw reply

* [PATCH 5/5] drm/sun4i: Add support for the overscan profiles
From: Daniel Vetter @ 2016-11-08  8:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <b980759ad766658d7bc3d67c18f505c5bf727ef5.1476779323.git-series.maxime.ripard@free-electrons.com>

On Tue, Oct 18, 2016 at 10:29:38AM +0200, Maxime Ripard wrote:
> Create overscan profiles reducing the displayed zone.
> 
> For each TV standard (PAL and NTSC so far), we create 4 more reduced modes
> by steps of 5% that the user will be able to select.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

tbh I think if we agree to do this (and that still seems an open question)
I think there should be a generic helper to add these overscan modes with
increased porches. Anything that only depends upon the sink (and
overscanning is something the sink does) should imo be put into a suitable
helper library for everyone to share.

Or maybe even stash it into the probe helpers and call it for all TV
connectors. Definitely not a driver-private thing.
-Daniel
> ---
>  drivers/gpu/drm/sun4i/sun4i_tv.c | 60 +++++++++++++++++++--------------
>  1 file changed, 36 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c b/drivers/gpu/drm/sun4i/sun4i_tv.c
> index f99886462cb8..9ee03ba086b6 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
> @@ -301,27 +301,33 @@ static const struct tv_mode *sun4i_tv_find_tv_by_mode(const struct drm_display_m
>  		DRM_DEBUG_DRIVER("Comparing mode %s vs %s",
>  				 mode->name, tv_mode->name);
>  
> -		if (!strcmp(mode->name, tv_mode->name))
> +		if (!strncmp(mode->name, tv_mode->name, strlen(tv_mode->name)))
>  			return tv_mode;
>  	}
>  
>  	/* Then by number of lines */
>  	for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
>  		const struct tv_mode *tv_mode = &tv_modes[i];
> +		int j;
>  
> -		DRM_DEBUG_DRIVER("Comparing mode %s vs %s (X: %d vs %d)",
> -				 mode->name, tv_mode->name,
> -				 mode->vdisplay, tv_mode->vdisplay);
> +		for (j = 0; j < 20; j += 5) {
> +			u32 vdisplay = tv_mode->vdisplay * (100 - j) / 100;
>  
> -		if (mode->vdisplay == tv_mode->vdisplay)
> -			return tv_mode;
> +			DRM_DEBUG_DRIVER("Comparing mode with %s (%d) (X: %d vs %d)",
> +					 tv_mode->name, j,
> +					 vdisplay, tv_mode->vdisplay);
> +
> +			if (vdisplay == tv_mode->vdisplay)
> +				return tv_mode;
> +		}
>  	}
>  
>  	return NULL;
>  }
>  
>  static void sun4i_tv_mode_to_drm_mode(const struct tv_mode *tv_mode,
> -				      struct drm_display_mode *mode)
> +				      struct drm_display_mode *mode,
> +				      int overscan)
>  {
>  	DRM_DEBUG_DRIVER("Creating mode %s\n", mode->name);
>  
> @@ -329,12 +335,12 @@ static void sun4i_tv_mode_to_drm_mode(const struct tv_mode *tv_mode,
>  	mode->clock = 13500;
>  	mode->flags = DRM_MODE_FLAG_INTERLACE;
>  
> -	mode->hdisplay = tv_mode->hdisplay;
> +	mode->hdisplay = tv_mode->hdisplay * (100 - overscan) / 100;
>  	mode->hsync_start = mode->hdisplay + tv_mode->hfront_porch;
>  	mode->hsync_end = mode->hsync_start + tv_mode->hsync_len;
>  	mode->htotal = mode->hsync_end  + tv_mode->hback_porch;
>  
> -	mode->vdisplay = tv_mode->vdisplay;
> +	mode->vdisplay = tv_mode->vdisplay * (100 - overscan) / 100;
>  	mode->vsync_start = mode->vdisplay + tv_mode->vfront_porch;
>  	mode->vsync_end = mode->vsync_start + tv_mode->vsync_len;
>  	mode->vtotal = mode->vsync_end  + tv_mode->vback_porch;
> @@ -352,10 +358,10 @@ static int sun4i_tv_atomic_check(struct drm_encoder *encoder,
>  		return -EINVAL;
>  
>  	state->display_x_size = tv_mode->hdisplay;
> -	state->plane_x_offset = 0;
> +	state->plane_x_offset = (tv_mode->hdisplay - mode->hdisplay) / 2;
>  
>  	state->display_y_size = tv_mode->vdisplay;
> -	state->plane_y_offset = 0;
> +	state->plane_y_offset = (tv_mode->vdisplay - mode->vdisplay) / 2;
>  
>  	return 0;
>  }
> @@ -404,7 +410,7 @@ static void sun4i_tv_mode_set(struct drm_encoder *encoder,
>  	struct drm_display_mode tv_drm_mode = { 0 };
>  
>  	strcpy(tv_drm_mode.name, "TV");
> -	sun4i_tv_mode_to_drm_mode(tv_mode, &tv_drm_mode);
> +	sun4i_tv_mode_to_drm_mode(tv_mode, &tv_drm_mode, 0);
>  	drm_mode_set_crtcinfo(&tv_drm_mode, CRTC_INTERLACE_HALVE_V);
>  
>  	sun4i_tcon1_mode_set(tcon, &tv_drm_mode);
> @@ -526,22 +532,28 @@ static int sun4i_tv_comp_get_modes(struct drm_connector *connector)
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
> -		struct drm_display_mode *mode;
>  		const struct tv_mode *tv_mode = &tv_modes[i];
> -
> -		mode = drm_mode_create(connector->dev);
> -		if (!mode) {
> -			DRM_ERROR("Failed to create a new display mode\n");
> -			return 0;
> +		int j;
> +
> +		for (j = 0; j < 20; j += 5) {
> +			struct drm_display_mode *mode = drm_mode_create(connector->dev);
> +			if (!mode) {
> +				DRM_ERROR("Failed to create a new display mode\n");
> +				return 0;
> +			}
> +
> +			if (j)
> +				sprintf(mode->name, "%s%d", tv_mode->name,
> +					j);
> +			else
> +				strcpy(mode->name, tv_mode->name);
> +
> +			sun4i_tv_mode_to_drm_mode(tv_mode, mode, j);
> +			drm_mode_probed_add(connector, mode);
>  		}
> -
> -		strcpy(mode->name, tv_mode->name);
> -
> -		sun4i_tv_mode_to_drm_mode(tv_mode, mode);
> -		drm_mode_probed_add(connector, mode);
>  	}
>  
> -	return i;
> +	return i * 4;
>  }
>  
>  static int sun4i_tv_comp_mode_valid(struct drm_connector *connector,
> -- 
> git-series 0.8.10
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply

* [PATCH] ARM: tegra: nyan: Mark all USB ports as host
From: Paul Kocialkowski @ 2016-11-08  9:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161108085420.GG2478@tbergstrom-lnx.Nvidia.com>

Le mardi 08 novembre 2016 ? 10:54 +0200, Peter De Schrijver a ?crit?:
> On Mon, Nov 07, 2016 at 02:09:31PM +0000, Jon Hunter wrote:
> > 
> > On 07/11/16 13:28, Thierry Reding wrote:
> > > * PGP Signed by an unknown key
> > > 
> > > On Sun, Sep 18, 2016 at 12:28:52PM +0200, Paul Kocialkowski wrote:
> > > > Nyan boards only have host USB ports (2 external, 1 internal), there is
> > > > no OTG-enabled connector.
> > > > 
> > > > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > > > ---
> > > > ?arch/arm/boot/dts/tegra124-nyan.dtsi | 2 +-
> > > > ?1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > Where is this information coming from? I don't have one of the Nyans
> > > myself, but one of the Tegra132 devices I have, which I think was
> > > derived from one of the Nyans uses one of the external host ports as
> > > forced recovery port, for which it would need OTG.
> > > 
> > > I suspect that the way to get U-Boot onto the Nyans is via tegrarcm?
> > > In that case I think one of the ports must be OTG.
> > 
> > It is true that the port on the back on the nyan-big can be used with
> > recovery mode. I was thinking that this is not a true OTG port as it is
> > just a 4-pin type A socket and does not have an ID pin. Thinking some
> > more about this the USB spec does include a "Host Negotiation Protocol
> > (HNP)" that allows a host and device to swap roles and so keeping it as
> > OTG seems valid afterall.
> 
> I don't think the bootrom implements that though. I expect recovery mode
> to just program the controller in device mode, without performing any
> negotiation.

That would make sense.

However, if there's a way (even not implemented yet, but a possible way) to have
the kernel configure this port as USB device instead of host dynamically (e.g.
without changing this bit in the dts), then I think it makes sense to keep the
OTG marking and drop this patch.

After all, switching to USB device mode doesn't necessarily have to come from
the ID pin.

-- 
Paul Kocialkowski, developer of free digital technology at the lower levels

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161108/66a1f1dd/attachment.sig>

^ permalink raw reply

* [PATCH v4] PM/devfreq: add suspend frequency support
From: Lin Huang @ 2016-11-08  9:11 UTC (permalink / raw)
  To: linux-arm-kernel

Add suspend frequency support and if needed set it to
the frequency obtained from the suspend opp (can be defined
using opp-v2 bindings and is optional).

Signed-off-by: Lin Huang <hl@rock-chips.com>
---
Changes in v2:
- use update_devfreq() instead devfreq_update_status()
Changes in v3:
- fix build error
Changes in v4:
- move dev_pm_opp_get_suspend_opp() to devfreq_add_device()

 drivers/devfreq/devfreq.c                 | 15 +++++++++++++--
 drivers/devfreq/governor_simpleondemand.c |  9 +++++++++
 include/linux/devfreq.h                   |  9 +++++++++
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index bf3ea76..d9d56e1 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -363,7 +363,10 @@ void devfreq_monitor_suspend(struct devfreq *devfreq)
 		mutex_unlock(&devfreq->lock);
 		return;
 	}
-
+	if (devfreq->suspend_freq) {
+		update_devfreq(devfreq);
+		devfreq->suspend_flag = true;
+	}
 	devfreq_update_status(devfreq, devfreq->previous_freq);
 	devfreq->stop_polling = true;
 	mutex_unlock(&devfreq->lock);
@@ -394,7 +397,8 @@ void devfreq_monitor_resume(struct devfreq *devfreq)
 
 	devfreq->last_stat_updated = jiffies;
 	devfreq->stop_polling = false;
-
+	if (devfreq->suspend_freq)
+		devfreq->suspend_flag = false;
 	if (devfreq->profile->get_cur_freq &&
 		!devfreq->profile->get_cur_freq(devfreq->dev.parent, &freq))
 		devfreq->previous_freq = freq;
@@ -528,6 +532,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	struct devfreq *devfreq;
 	struct devfreq_governor *governor;
 	int err = 0;
+	struct dev_pm_opp *suspend_opp;
 
 	if (!dev || !profile || !governor_name) {
 		dev_err(dev, "%s: Invalid parameters.\n", __func__);
@@ -563,6 +568,12 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	devfreq->data = data;
 	devfreq->nb.notifier_call = devfreq_notifier_call;
 
+	rcu_read_lock();
+	suspend_opp = dev_pm_opp_get_suspend_opp(dev);
+	if (suspend_opp)
+		devfreq->suspend_freq = dev_pm_opp_get_freq(suspend_opp);
+	rcu_read_unlock();
+
 	if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
 		mutex_unlock(&devfreq->lock);
 		devfreq_set_freq_table(devfreq);
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
index ae72ba5..84b3ce1 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -29,6 +29,15 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
 	struct devfreq_simple_ondemand_data *data = df->data;
 	unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
 
+	/*
+	 * if devfreq in suspend status and have suspend_freq,
+	 * the frequency need to set to suspend_freq
+	 */
+	if (df->suspend_flag) {
+		*freq =	df->suspend_freq;
+		return 0;
+	}
+
 	err = devfreq_update_stats(df);
 	if (err)
 		return err;
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 2de4e2e..c463ae1 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -172,6 +172,7 @@ struct devfreq {
 	struct delayed_work work;
 
 	unsigned long previous_freq;
+	unsigned long suspend_freq;
 	struct devfreq_dev_status last_status;
 
 	void *data; /* private data for governors */
@@ -179,6 +180,7 @@ struct devfreq {
 	unsigned long min_freq;
 	unsigned long max_freq;
 	bool stop_polling;
+	bool suspend_flag;
 
 	/* information for device frequency transition */
 	unsigned int total_trans;
@@ -214,6 +216,8 @@ extern int devfreq_resume_device(struct devfreq *devfreq);
 /* Helper functions for devfreq user device driver with OPP. */
 extern struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
 					   unsigned long *freq, u32 flags);
+extern void devfreq_opp_get_suspend_opp(struct device *dev,
+					struct devfreq *devfreq);
 extern int devfreq_register_opp_notifier(struct device *dev,
 					 struct devfreq *devfreq);
 extern int devfreq_unregister_opp_notifier(struct device *dev,
@@ -348,6 +352,11 @@ static inline struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
 	return ERR_PTR(-EINVAL);
 }
 
+static inline void devfreq_opp_get_suspend_opp(struct device *dev,
+					       struct devfreq *devfreq)
+{
+}
+
 static inline int devfreq_register_opp_notifier(struct device *dev,
 					 struct devfreq *devfreq)
 {
-- 
2.6.6

^ permalink raw reply related

* [PATCH 01/30] usb: dwc2: Deprecate g-use-dma binding
From: Felipe Balbi @ 2016-11-08  9:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e99e9d8df77884671f94734109a1a8d76a0222c1.1478558343.git.johnyoun@synopsys.com>


Hi,

John Youn <johnyoun@synopsys.com> writes:
> Add a vendor prefix and make the name more consistent by renaming it to
> "snps,gadget-dma-enable".
>
> Signed-off-by: John Youn <johnyoun@synopsys.com>
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
>  arch/arm/boot/dts/rk3036.dtsi                  | 2 +-
>  arch/arm/boot/dts/rk3288.dtsi                  | 2 +-
>  arch/arm/boot/dts/rk3xxx.dtsi                  | 2 +-
>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi      | 2 +-
>  arch/arm64/boot/dts/rockchip/rk3368.dtsi       | 2 +-
>  drivers/usb/dwc2/params.c                      | 9 ++++++++-
>  drivers/usb/dwc2/pci.c                         | 2 +-
>  8 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> index 9472111..389a461 100644
> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> @@ -26,11 +26,14 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
>  - dr_mode: shall be one of "host", "peripheral" and "otg"
>    Refer to usb/generic.txt
>  - snps,host-dma-disable: disable host DMA mode.
> -- g-use-dma: enable dma usage in gadget driver.
> +- snps,gadget-dma-enable: enable gadget DMA mode.

I don't see why you even have this binding. Looking through the code,
you have:

#define GHWCFG2_SLAVE_ONLY_ARCH			0
#define GHWCFG2_EXT_DMA_ARCH			1
#define GHWCFG2_INT_DMA_ARCH			2

void dwc2_set_param_dma_enable(struct dwc2_hsotg *hsotg, int val)
{
	int valid = 1;

	if (val > 0 && hsotg->hw_params.arch == GHWCFG2_SLAVE_ONLY_ARCH)
		valid = 0;
	if (val < 0)
		valid = 0;

	if (!valid) {
		if (val >= 0)
			dev_err(hsotg->dev,
				"%d invalid for dma_enable parameter. Check HW configuration.\n",
				val);
		val = hsotg->hw_params.arch != GHWCFG2_SLAVE_ONLY_ARCH;
		dev_dbg(hsotg->dev, "Setting dma_enable to %d\n", val);
	}

	hsotg->core_params->dma_enable = val;
}

which seems to hint that DMA support is discoverable. If there is DMA,
why would disable it?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161108/28908c8d/attachment-0001.sig>

^ permalink raw reply

* [PATCH 2/2] mm: hugetlb: support gigantic surplus pages
From: Huang Shijie @ 2016-11-08  9:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161108070851.GA15044@sha-win-210.asiapac.arm.com>

Hi Gerald,
On Tue, Nov 08, 2016 at 03:08:52PM +0800, Huang Shijie wrote:
> On Tue, Nov 08, 2016 at 10:19:30AM +0800, Huang Shijie wrote:
> > On Mon, Nov 07, 2016 at 04:25:04PM +0100, Gerald Schaefer wrote:
> > > On Thu, 3 Nov 2016 10:51:38 +0800
> > > Huang Shijie <shijie.huang@arm.com> wrote:
> > > 
> > > > When testing the gigantic page whose order is too large for the buddy
> > > > allocator, the libhugetlbfs test case "counter.sh" will fail.
> > > > 
> > > > The failure is caused by:
> > > >  1) kernel fails to allocate a gigantic page for the surplus case.
> > > >     And the gather_surplus_pages() will return NULL in the end.
> > > > 
> > > >  2) The condition checks for "over-commit" is wrong.
> > > > 
> > > > This patch adds code to allocate the gigantic page in the
> > > > __alloc_huge_page(). After this patch, gather_surplus_pages()
> > > > can return a gigantic page for the surplus case.
> > > > 
> > > > This patch also changes the condition checks for:
> > > >      return_unused_surplus_pages()
> > > >      nr_overcommit_hugepages_store()
> > > > 
> > > > After this patch, the counter.sh can pass for the gigantic page.
> > > > 
> > > > Acked-by: Steve Capper <steve.capper@arm.com>
> > > > Signed-off-by: Huang Shijie <shijie.huang@arm.com>
> > > > ---
> > > >  mm/hugetlb.c | 15 ++++++++++-----
> > > >  1 file changed, 10 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > > > index 0bf4444..2b67aff 100644
> > > > --- a/mm/hugetlb.c
> > > > +++ b/mm/hugetlb.c
> > > > @@ -1574,7 +1574,7 @@ static struct page *__alloc_huge_page(struct hstate *h,
> > > >  	struct page *page;
> > > >  	unsigned int r_nid;
> > > > 
> > > > -	if (hstate_is_gigantic(h))
> > > > +	if (hstate_is_gigantic(h) && !gigantic_page_supported())
> > > >  		return NULL;
> > > 
> > > Is it really possible to stumble over gigantic pages w/o having
> > > gigantic_page_supported()?
> > > 
> > > Also, I've just tried this on s390 and counter.sh still fails after these
> > > patches, and it should fail on all archs as long as you use the gigantic
> > I guess the failure you met is caused by the libhugetlbfs itself, there are
> > several bugs in the libhugetlbfs. I have a patch set for the
> > libhugetlbfs too. I will send it as soon as possible.
> > 
> > > hugepage size as default hugepage size. This is because you only changed
> > > nr_overcommit_hugepages_store(), which handles nr_overcommit_hugepages
> > > in sysfs, and missed hugetlb_overcommit_handler() which handles
> > > /proc/sys/vm/nr_overcommit_hugepages for the default sized hugepages.
> > This is wrong. :)
> Sorry, I was wrong :). The counters test does call the /proc/sys/vm/nr_overcommit_hugepages.
> But in the arm64, it does not trigger a fail for the counters test.
> In an other word, I did not change the hugetlb_overcommit_handler(),
> the counters.sh also can pass in arm64. 
After I add the "default_hugepagesz=32M" to the kernel cmdlin, I can
reproduce this issue. Thanks for point this.

> 
> I will look at the lockdep issue.
I tested the new patch (will be sent out later) on the arm64 platform,
and I did not meet the lockdep issue when I enabled the lockdep.
The following is my config:

	CONFIG_LOCKD=y
	CONFIG_LOCKD_V4=y
	CONFIG_LOCKUP_DETECTOR=y
        # CONFIG_BOOTPARAM_SOFTLOCKUP_PANIC is not set
	CONFIG_BOOTPARAM_SOFTLOCKUP_PANIC_VALUE=0
	CONFIG_DEBUG_SPINLOCK=y
	CONFIG_DEBUG_LOCK_ALLOC=y
	CONFIG_PROVE_LOCKING=y
	CONFIG_LOCKDEP=y
	CONFIG_LOCK_STAT=y
	CONFIG_DEBUG_LOCKDEP=y
	CONFIG_DEBUG_LOCKING_API_SELFTESTS=y
	
So do I miss something? 

Thanks	
Huang Shijie

^ permalink raw reply

* [PATCH] pinctrl: sx150x: fix up headers
From: Linus Walleij @ 2016-11-08  9:27 UTC (permalink / raw)
  To: linux-arm-kernel

Include <linux/gpio/driver.h> rather than <linux/gpio.h>
Drop <linux/pinctrl/machine.h>.

Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/pinctrl/pinctrl-sx150x.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-sx150x.c b/drivers/pinctrl/pinctrl-sx150x.c
index dc1341fdc73d..63778058eec7 100644
--- a/drivers/pinctrl/pinctrl-sx150x.c
+++ b/drivers/pinctrl/pinctrl-sx150x.c
@@ -27,8 +27,7 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
-#include <linux/gpio.h>
-#include <linux/pinctrl/machine.h>
+#include <linux/gpio/driver.h>
 #include <linux/pinctrl/pinconf.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
-- 
2.7.4

^ permalink raw reply related

* [PATCH v4] PM/devfreq: add suspend frequency support
From: Chanwoo Choi @ 2016-11-08  9:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478596289-7442-1-git-send-email-hl@rock-chips.com>

Hi Lin,

On 2016? 11? 08? 18:11, Lin Huang wrote:
> Add suspend frequency support and if needed set it to
> the frequency obtained from the suspend opp (can be defined
> using opp-v2 bindings and is optional).
> 
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> Changes in v2:
> - use update_devfreq() instead devfreq_update_status()
> Changes in v3:
> - fix build error
> Changes in v4:
> - move dev_pm_opp_get_suspend_opp() to devfreq_add_device()
> 
>  drivers/devfreq/devfreq.c                 | 15 +++++++++++++--
>  drivers/devfreq/governor_simpleondemand.c |  9 +++++++++
>  include/linux/devfreq.h                   |  9 +++++++++
>  3 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index bf3ea76..d9d56e1 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -363,7 +363,10 @@ void devfreq_monitor_suspend(struct devfreq *devfreq)
>  		mutex_unlock(&devfreq->lock);
>  		return;
>  	}
> -
> +	if (devfreq->suspend_freq) {
> +		update_devfreq(devfreq);
> +		devfreq->suspend_flag = true;

You don't need the additional variable (devfreq->suspend_flag).
When adding the devfreq on devfreq_add_device(),
you can initialize the devfreq->suspend_freq as zero(0).

You can check whether devfreq->suspend_freq is 0 or not
without the new suspend_flag.

> +	}
>  	devfreq_update_status(devfreq, devfreq->previous_freq);
>  	devfreq->stop_polling = true;
>  	mutex_unlock(&devfreq->lock);
> @@ -394,7 +397,8 @@ void devfreq_monitor_resume(struct devfreq *devfreq)
>  
>  	devfreq->last_stat_updated = jiffies;
>  	devfreq->stop_polling = false;
> -
> +	if (devfreq->suspend_freq)
> +		devfreq->suspend_flag = false;

ditto. You don't need to add this code.

>  	if (devfreq->profile->get_cur_freq &&
>  		!devfreq->profile->get_cur_freq(devfreq->dev.parent, &freq))
>  		devfreq->previous_freq = freq;
> @@ -528,6 +532,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  	struct devfreq *devfreq;
>  	struct devfreq_governor *governor;
>  	int err = 0;
> +	struct dev_pm_opp *suspend_opp;
>  
>  	if (!dev || !profile || !governor_name) {
>  		dev_err(dev, "%s: Invalid parameters.\n", __func__);
> @@ -563,6 +568,12 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  	devfreq->data = data;
>  	devfreq->nb.notifier_call = devfreq_notifier_call;
>  
> +	rcu_read_lock();
> +	suspend_opp = dev_pm_opp_get_suspend_opp(dev);
> +	if (suspend_opp)
> +		devfreq->suspend_freq = dev_pm_opp_get_freq(suspend_opp);
> +	rcu_read_unlock();
> +
>  	if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
>  		mutex_unlock(&devfreq->lock);
>  		devfreq_set_freq_table(devfreq);
> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> index ae72ba5..84b3ce1 100644
> --- a/drivers/devfreq/governor_simpleondemand.c
> +++ b/drivers/devfreq/governor_simpleondemand.c
> @@ -29,6 +29,15 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>  	struct devfreq_simple_ondemand_data *data = df->data;
>  	unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
>  
> +	/*
> +	 * if devfreq in suspend status and have suspend_freq,
> +	 * the frequency need to set to suspend_freq
> +	 */
> +	if (df->suspend_flag) {
> +		*freq =	df->suspend_freq;
> +		return 0;
> +	}

You can check it as following:

	if (df->suspend_freq != 0)
		*freq = df->suspend_freq;

> +
>  	err = devfreq_update_stats(df);
>  	if (err)
>  		return err;
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 2de4e2e..c463ae1 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -172,6 +172,7 @@ struct devfreq {
>  	struct delayed_work work;
>  
>  	unsigned long previous_freq;
> +	unsigned long suspend_freq;
>  	struct devfreq_dev_status last_status;
>  
>  	void *data; /* private data for governors */
> @@ -179,6 +180,7 @@ struct devfreq {
>  	unsigned long min_freq;
>  	unsigned long max_freq;
>  	bool stop_polling;
> +	bool suspend_flag;

You don't need to add new variable.

>  
>  	/* information for device frequency transition */
>  	unsigned int total_trans;
> @@ -214,6 +216,8 @@ extern int devfreq_resume_device(struct devfreq *devfreq);
>  /* Helper functions for devfreq user device driver with OPP. */
>  extern struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
>  					   unsigned long *freq, u32 flags);
> +extern void devfreq_opp_get_suspend_opp(struct device *dev,
> +					struct devfreq *devfreq);

Why do you need this function? Also, this patch don't include the body
of devfreq_opp_get_suspend_opp() function.

>  extern int devfreq_register_opp_notifier(struct device *dev,
>  					 struct devfreq *devfreq);
>  extern int devfreq_unregister_opp_notifier(struct device *dev,
> @@ -348,6 +352,11 @@ static inline struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
>  	return ERR_PTR(-EINVAL);
>  }
>  
> +static inline void devfreq_opp_get_suspend_opp(struct device *dev,
> +					       struct devfreq *devfreq)
> +{
> +}
> +
>  static inline int devfreq_register_opp_notifier(struct device *dev,
>  					 struct devfreq *devfreq)
>  {
> 

-- 
Best Regards,
Chanwoo Choi

^ permalink raw reply

* [PATCH v3 1/5] pinctrl: samsung: Add the support the multiple IORESOURCE_MEM for one pin-bank
From: Linus Walleij @ 2016-11-08  9:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478155149-28527-2-git-send-email-cw00.choi@samsung.com>

On Thu, Nov 3, 2016 at 7:39 AM, Chanwoo Choi <cw00.choi@samsung.com> wrote:

> This patch supports the multiple IORESOURCE_MEM resources for one pin-bank.
> In the pre-existing Exynos series, the registers of the gpio bank are included
> in the one memory map. But, some gpio bank need to support the one more memory
> map (IORESOURCE_MEM) because the registers of gpio bank are separated into
> the different memory map.
>
> For example,
> The both ALIVE and IMEM domain have the different memory base address.
> The GFP[1-5] of exynos5433 are composed as following:
> - ALIVE domain : WEINT_* registers
> - IMEM domain  : CON/DAT/PUD/DRV/CONPDN/PUDPDN register
>
> Cc: Tomasz Figa <tomasz.figa@gmail.com>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Cc: Kukjin Kim <kgene@kernel.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-gpio at vger.kernel.org
> Suggested-by: Tomasz Figa <tomasz.figa@gmail.com>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>

Patch applied with Krzysztof's review tag.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v3 2/5] pinctrl: samsung: Add GPF support for Exynos5433
From: Linus Walleij @ 2016-11-08  9:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478155149-28527-3-git-send-email-cw00.choi@samsung.com>

On Thu, Nov 3, 2016 at 7:39 AM, Chanwoo Choi <cw00.choi@samsung.com> wrote:

> This patch add the support of GPF[1-5] pin of Exynos5433 SoC. The GPFx need
> to support the multiple memory map because the registers of GPFx are located
> in the different domain.
>
> Cc: Tomasz Figa <tomasz.figa@gmail.com>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Cc: Kukjin Kim <kgene@kernel.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-gpio at vger.kernel.org
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>

Patch applied.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v3 2/5] pinctrl: samsung: Add GPF support for Exynos5433
From: Linus Walleij @ 2016-11-08  9:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161103192002.GB12945@kozik-lap>

On Thu, Nov 3, 2016 at 8:20 PM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Thu, Nov 03, 2016 at 03:39:06PM +0900, Chanwoo Choi wrote:
>> This patch add the support of GPF[1-5] pin of Exynos5433 SoC. The GPFx need
>> to support the multiple memory map because the registers of GPFx are located
>> in the different domain.
>>
>> Cc: Tomasz Figa <tomasz.figa@gmail.com>
>> Cc: Krzysztof Kozlowski <krzk@kernel.org>
>> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Cc: Kukjin Kim <kgene@kernel.org>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: linux-gpio at vger.kernel.org
>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  drivers/pinctrl/samsung/pinctrl-exynos.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>
> I think that, instead of in previous patch, the
> "samsung,exynos5433-pinctrl" compatible should be documented here along
> with information that it requires two addresses for mappings.

True but too small detail to respin the patches about,
and I'm not perfectionist, so patch applied anyways.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH] ARM: tegra: nyan: Mark all USB ports as host
From: Jon Hunter @ 2016-11-08  9:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161108085420.GG2478@tbergstrom-lnx.Nvidia.com>


On 08/11/16 08:54, Peter De Schrijver wrote:
> On Mon, Nov 07, 2016 at 02:09:31PM +0000, Jon Hunter wrote:
>>
>> On 07/11/16 13:28, Thierry Reding wrote:
>>> * PGP Signed by an unknown key
>>>
>>> On Sun, Sep 18, 2016 at 12:28:52PM +0200, Paul Kocialkowski wrote:
>>>> Nyan boards only have host USB ports (2 external, 1 internal), there is
>>>> no OTG-enabled connector.
>>>>
>>>> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
>>>> ---
>>>>  arch/arm/boot/dts/tegra124-nyan.dtsi | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> Where is this information coming from? I don't have one of the Nyans
>>> myself, but one of the Tegra132 devices I have, which I think was
>>> derived from one of the Nyans uses one of the external host ports as
>>> forced recovery port, for which it would need OTG.
>>>
>>> I suspect that the way to get U-Boot onto the Nyans is via tegrarcm?
>>> In that case I think one of the ports must be OTG.
>>
>> It is true that the port on the back on the nyan-big can be used with
>> recovery mode. I was thinking that this is not a true OTG port as it is
>> just a 4-pin type A socket and does not have an ID pin. Thinking some
>> more about this the USB spec does include a "Host Negotiation Protocol
>> (HNP)" that allows a host and device to swap roles and so keeping it as
>> OTG seems valid afterall.
> 
> I don't think the bootrom implements that though. I expect recovery mode
> to just program the controller in device mode, without performing any
> negotiation.

I am not talking about the bootrom and I would not expect the bootrom to
do that. However, the kernel could.

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* [PATCH v2 2/2] mmc: sdhci-iproc: support standard byte register accesses
From: Adrian Hunter @ 2016-11-08  9:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478018277-10097-3-git-send-email-scott.branden@broadcom.com>

On 01/11/16 18:37, Scott Branden wrote:
> Add bytewise register accesses support for newer versions of IPROC
> SDHCI controllers.
> Previous sdhci-iproc versions of SDIO controllers
> (such as Raspberry Pi and Cygnus) only allowed for 32-bit register
> accesses.
> 
> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>

This is unchanged from V1 which I acked, so:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

^ permalink raw reply

* [PATCH v4] PM/devfreq: add suspend frequency support
From: hl @ 2016-11-08  9:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <58219B96.6060106@samsung.com>

Hi Chanwoo Choi,

On 2016?11?08? 17:32, Chanwoo Choi wrote:
> Hi Lin,
>
> On 2016? 11? 08? 18:11, Lin Huang wrote:
>> Add suspend frequency support and if needed set it to
>> the frequency obtained from the suspend opp (can be defined
>> using opp-v2 bindings and is optional).
>>
>> Signed-off-by: Lin Huang <hl@rock-chips.com>
>> ---
>> Changes in v2:
>> - use update_devfreq() instead devfreq_update_status()
>> Changes in v3:
>> - fix build error
>> Changes in v4:
>> - move dev_pm_opp_get_suspend_opp() to devfreq_add_device()
>>
>>   drivers/devfreq/devfreq.c                 | 15 +++++++++++++--
>>   drivers/devfreq/governor_simpleondemand.c |  9 +++++++++
>>   include/linux/devfreq.h                   |  9 +++++++++
>>   3 files changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index bf3ea76..d9d56e1 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -363,7 +363,10 @@ void devfreq_monitor_suspend(struct devfreq *devfreq)
>>   		mutex_unlock(&devfreq->lock);
>>   		return;
>>   	}
>> -
>> +	if (devfreq->suspend_freq) {
>> +		update_devfreq(devfreq);
>> +		devfreq->suspend_flag = true;
> You don't need the additional variable (devfreq->suspend_flag).
> When adding the devfreq on devfreq_add_device(),
> you can initialize the devfreq->suspend_freq as zero(0).
>
> You can check whether devfreq->suspend_freq is 0 or not
> without the new suspend_flag.
>
>> +	}
>>   	devfreq_update_status(devfreq, devfreq->previous_freq);
>>   	devfreq->stop_polling = true;
>>   	mutex_unlock(&devfreq->lock);
>> @@ -394,7 +397,8 @@ void devfreq_monitor_resume(struct devfreq *devfreq)
>>   
>>   	devfreq->last_stat_updated = jiffies;
>>   	devfreq->stop_polling = false;
>> -
>> +	if (devfreq->suspend_freq)
>> +		devfreq->suspend_flag = false;
> ditto. You don't need to add this code.
>
>>   	if (devfreq->profile->get_cur_freq &&
>>   		!devfreq->profile->get_cur_freq(devfreq->dev.parent, &freq))
>>   		devfreq->previous_freq = freq;
>> @@ -528,6 +532,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>   	struct devfreq *devfreq;
>>   	struct devfreq_governor *governor;
>>   	int err = 0;
>> +	struct dev_pm_opp *suspend_opp;
>>   
>>   	if (!dev || !profile || !governor_name) {
>>   		dev_err(dev, "%s: Invalid parameters.\n", __func__);
>> @@ -563,6 +568,12 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>   	devfreq->data = data;
>>   	devfreq->nb.notifier_call = devfreq_notifier_call;
>>   
>> +	rcu_read_lock();
>> +	suspend_opp = dev_pm_opp_get_suspend_opp(dev);
>> +	if (suspend_opp)
>> +		devfreq->suspend_freq = dev_pm_opp_get_freq(suspend_opp);
>> +	rcu_read_unlock();
>> +
>>   	if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
>>   		mutex_unlock(&devfreq->lock);
>>   		devfreq_set_freq_table(devfreq);
>> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
>> index ae72ba5..84b3ce1 100644
>> --- a/drivers/devfreq/governor_simpleondemand.c
>> +++ b/drivers/devfreq/governor_simpleondemand.c
>> @@ -29,6 +29,15 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>>   	struct devfreq_simple_ondemand_data *data = df->data;
>>   	unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
>>   
>> +	/*
>> +	 * if devfreq in suspend status and have suspend_freq,
>> +	 * the frequency need to set to suspend_freq
>> +	 */
>> +	if (df->suspend_flag) {
>> +		*freq =	df->suspend_freq;
>> +		return 0;
>> +	}
> You can check it as following:
>
> 	if (df->suspend_freq != 0)
> 		*freq = df->suspend_freq;
If i do like this, it will always return suspend frequency, since 
devfreq->suspend_freq will be assigned value
if we define it on dts. But what we want is only in suspend status we 
return the suspend frequency.
>
>> +
>>   	err = devfreq_update_stats(df);
>>   	if (err)
>>   		return err;
>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>> index 2de4e2e..c463ae1 100644
>> --- a/include/linux/devfreq.h
>> +++ b/include/linux/devfreq.h
>> @@ -172,6 +172,7 @@ struct devfreq {
>>   	struct delayed_work work;
>>   
>>   	unsigned long previous_freq;
>> +	unsigned long suspend_freq;
>>   	struct devfreq_dev_status last_status;
>>   
>>   	void *data; /* private data for governors */
>> @@ -179,6 +180,7 @@ struct devfreq {
>>   	unsigned long min_freq;
>>   	unsigned long max_freq;
>>   	bool stop_polling;
>> +	bool suspend_flag;
> You don't need to add new variable.
>
>>   
>>   	/* information for device frequency transition */
>>   	unsigned int total_trans;
>> @@ -214,6 +216,8 @@ extern int devfreq_resume_device(struct devfreq *devfreq);
>>   /* Helper functions for devfreq user device driver with OPP. */
>>   extern struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
>>   					   unsigned long *freq, u32 flags);
>> +extern void devfreq_opp_get_suspend_opp(struct device *dev,
>> +					struct devfreq *devfreq);
> Why do you need this function? Also, this patch don't include the body
> of devfreq_opp_get_suspend_opp() function.
Oh, forget to delete it, sorry about that.
>
>>   extern int devfreq_register_opp_notifier(struct device *dev,
>>   					 struct devfreq *devfreq);
>>   extern int devfreq_unregister_opp_notifier(struct device *dev,
>> @@ -348,6 +352,11 @@ static inline struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
>>   	return ERR_PTR(-EINVAL);
>>   }
>>   
>> +static inline void devfreq_opp_get_suspend_opp(struct device *dev,
>> +					       struct devfreq *devfreq)
>> +{
>> +}
>> +
>>   static inline int devfreq_register_opp_notifier(struct device *dev,
>>   					 struct devfreq *devfreq)
>>   {
>>

-- 
Lin Huang

^ permalink raw reply

* [PATCH 2/2] irqchip/gicv3-its: Test code for measuring Read-allocate
From: kbuild test robot @ 2016-11-08  9:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8c3ea714-5bd2-6784-8eae-8b953860c8f6@codeaurora.org>

Hi Shanker,

[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on v4.9-rc4 next-20161028]
[cannot apply to tip/irq/core arm-jcooper/irqchip/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Shanker-Donthineni/irqchip-gicv3-its-Test-code-for-measuring-Read-allocate/20161108-154723
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm-multi_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-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
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   /tmp/ccEUFgcT.s: Assembler messages:
>> /tmp/ccEUFgcT.s:700: Error: selected processor does not support requested special purpose register -- `mrs r10,pmccntr_el0'

---
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: 38721 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161108/f74b1c64/attachment-0001.gz>

^ permalink raw reply

* [PATCH] fpga zynq: Check the bitstream for validity
From: Matthias Brugger @ 2016-11-08  9:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161108004642.GB32444@obsidianresearch.com>



On 11/08/2016 01:46 AM, Jason Gunthorpe wrote:
> On Fri, Oct 28, 2016 at 05:09:26PM -0700, Moritz Fischer wrote:
>
>> That being said, I don't like the idea of the driver having to search
>> either...
>
> I think we are stuck with that, considering what Xilinx tools
> produce..
>
> Here is a v2, what do you think?
>
> From 93ffde371ca50809ba9b4cdca17051a050b0f92d Mon Sep 17 00:00:00 2001
> From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Date: Wed, 26 Oct 2016 16:51:26 -0600
> Subject: [PATCH v2] fpga zynq: Check the bitstream for validity
>
> There is no sense in sending a bitstream we know will not work, and
> with the variety of options for bitstream generation in Xilinx tools
> it is not terribly clear or very well documented what the correct
> input should be, especially since auto-detection was removed from this
> driver.
>
> All Zynq full configuration bitstreams must start with the sync word in
> the correct byte order.
>
> Zynq is also only able to DMA dword quantities, so bitstreams must be
> a multiple of 4 bytes. This also fixes a DMA-past the end bug.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  drivers/fpga/zynq-fpga.c | 35 ++++++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
> index c2fb4120bd62..de475a6a1882 100644
> --- a/drivers/fpga/zynq-fpga.c
> +++ b/drivers/fpga/zynq-fpga.c
> @@ -175,6 +175,19 @@ static irqreturn_t zynq_fpga_isr(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>
> +/* Sanity check the proposed bitstream. It must start with the sync word in
> + * the correct byte order. The input is a Xilinx .bin file with every 32 bit
> + * quantity swapped.
> + */
> +static bool zynq_fpga_has_sync(const char *buf, size_t count)
> +{
> +	for (; count > 4; buf += 4, --count)
> +		if (buf[0] == 0x66 && buf[1] == 0x55 && buf[2] == 0x99 &&
> +		    buf[3] == 0xaa)
> +			return true;
> +	return false;
> +}
> +
>  static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags,
>  				    const char *buf, size_t count)
>  {
> @@ -184,12 +197,23 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags,
>
>  	priv = mgr->priv;
>
> +	/* The hardware can only DMA multiples of 4 bytes, and we need at
> +	 * least the sync word and something else to do anything.
> +	 */
> +	if (count <= 4 || (count % 4) != 0)
> +		return -EINVAL;
> +
>  	err = clk_enable(priv->clk);
>  	if (err)
>  		return err;
>
>  	/* don't globally reset PL if we're doing partial reconfig */
>  	if (!(flags & FPGA_MGR_PARTIAL_RECONFIG)) {
> +		if (!zynq_fpga_has_sync(buf, count)) {

Maybe we should add an error message here to let the user know what went 
wrong, as I think this error path could easily be hit by an user.

Regards,
Matthias


> +			err = -EINVAL;
> +			goto out_err;
> +		}
> +
>  		/* assert AXI interface resets */
>  		regmap_write(priv->slcr, SLCR_FPGA_RST_CTRL_OFFSET,
>  			     FPGA_RST_ALL_MASK);
> @@ -287,12 +311,9 @@ static int zynq_fpga_ops_write(struct fpga_manager *mgr,
>  	struct zynq_fpga_priv *priv;
>  	int err;
>  	char *kbuf;
> -	size_t in_count;
>  	dma_addr_t dma_addr;
> -	u32 transfer_length;
>  	u32 intr_status;
>
> -	in_count = count;
>  	priv = mgr->priv;
>
>  	kbuf = dma_alloc_coherent(priv->dev, count, &dma_addr, GFP_KERNEL);
> @@ -318,11 +339,7 @@ static int zynq_fpga_ops_write(struct fpga_manager *mgr,
>  	 */
>  	zynq_fpga_write(priv, DMA_SRC_ADDR_OFFSET, (u32)(dma_addr) + 1);
>  	zynq_fpga_write(priv, DMA_DST_ADDR_OFFSET, (u32)DMA_INVALID_ADDRESS);
> -
> -	/* convert #bytes to #words */
> -	transfer_length = (count + 3) / 4;
> -
> -	zynq_fpga_write(priv, DMA_SRC_LEN_OFFSET, transfer_length);
> +	zynq_fpga_write(priv, DMA_SRC_LEN_OFFSET, count / 4);
>  	zynq_fpga_write(priv, DMA_DEST_LEN_OFFSET, 0);
>
>  	wait_for_completion(&priv->dma_done);
> @@ -338,7 +355,7 @@ static int zynq_fpga_ops_write(struct fpga_manager *mgr,
>  	clk_disable(priv->clk);
>
>  out_free:
> -	dma_free_coherent(priv->dev, in_count, kbuf, dma_addr);
> +	dma_free_coherent(priv->dev, count, kbuf, dma_addr);
>
>  	return err;
>  }
>

^ permalink raw reply

* [PATCH v5] PM/devfreq: add suspend frequency support
From: Lin Huang @ 2016-11-08 10:06 UTC (permalink / raw)
  To: linux-arm-kernel

Add suspend frequency support and if needed set it to
the frequency obtained from the suspend opp (can be defined
using opp-v2 bindings and is optional).

Signed-off-by: Lin Huang <hl@rock-chips.com>
---
Changes in v2:
- use update_devfreq() instead devfreq_update_status()
Changes in v3:
- fix build error
Changes in v4:
- move dev_pm_opp_get_suspend_opp() to devfreq_add_device()
Changes in v5:
- delete devfreq_opp_get_suspend_opp() in devfreq.h

 drivers/devfreq/devfreq.c                 | 15 +++++++++++++--
 drivers/devfreq/governor_simpleondemand.c |  9 +++++++++
 include/linux/devfreq.h                   |  2 ++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index da72d97..af06891 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -359,7 +359,10 @@ void devfreq_monitor_suspend(struct devfreq *devfreq)
 		mutex_unlock(&devfreq->lock);
 		return;
 	}
-
+	if (devfreq->suspend_freq) {
+		update_devfreq(devfreq);
+		devfreq->suspend_flag = true;
+	}
 	devfreq_update_status(devfreq, devfreq->previous_freq);
 	devfreq->stop_polling = true;
 	mutex_unlock(&devfreq->lock);
@@ -390,7 +393,8 @@ void devfreq_monitor_resume(struct devfreq *devfreq)
 
 	devfreq->last_stat_updated = jiffies;
 	devfreq->stop_polling = false;
-
+	if (devfreq->suspend_freq)
+		devfreq->suspend_flag = false;
 	if (devfreq->profile->get_cur_freq &&
 		!devfreq->profile->get_cur_freq(devfreq->dev.parent, &freq))
 		devfreq->previous_freq = freq;
@@ -524,6 +528,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	struct devfreq *devfreq;
 	struct devfreq_governor *governor;
 	int err = 0;
+	struct dev_pm_opp *suspend_opp;
 
 	if (!dev || !profile || !governor_name) {
 		dev_err(dev, "%s: Invalid parameters.\n", __func__);
@@ -558,6 +563,12 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	devfreq->data = data;
 	devfreq->nb.notifier_call = devfreq_notifier_call;
 
+	rcu_read_lock();
+	suspend_opp = dev_pm_opp_get_suspend_opp(dev);
+	if (suspend_opp)
+		devfreq->suspend_freq = dev_pm_opp_get_freq(suspend_opp);
+	rcu_read_unlock();
+
 	if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
 		mutex_unlock(&devfreq->lock);
 		devfreq_set_freq_table(devfreq);
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
index ae72ba5..84b3ce1 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -29,6 +29,15 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
 	struct devfreq_simple_ondemand_data *data = df->data;
 	unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
 
+	/*
+	 * if devfreq in suspend status and have suspend_freq,
+	 * the frequency need to set to suspend_freq
+	 */
+	if (df->suspend_flag) {
+		*freq =	df->suspend_freq;
+		return 0;
+	}
+
 	err = devfreq_update_stats(df);
 	if (err)
 		return err;
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 98c6993..8567153 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -172,6 +172,7 @@ struct devfreq {
 	struct delayed_work work;
 
 	unsigned long previous_freq;
+	unsigned long suspend_freq;
 	struct devfreq_dev_status last_status;
 
 	void *data; /* private data for governors */
@@ -179,6 +180,7 @@ struct devfreq {
 	unsigned long min_freq;
 	unsigned long max_freq;
 	bool stop_polling;
+	bool suspend_flag;
 
 	/* information for device frequency transition */
 	unsigned int total_trans;
-- 
2.6.6

^ permalink raw reply related

* [PATCH 1/1] pinctrl: st: st_pinconf_dbg_show wrong format string
From: Linus Walleij @ 2016-11-08 10:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161105142515.4377-1-xypron.glpk@gmx.de>

On Sat, Nov 5, 2016 at 3:25 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> function is defined as unsigned int.
> So we need %u to print it.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Patch applied with Patrice's ACK.

Yours,
Linus Walleij

^ permalink raw reply


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