Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/3] arm64: dump: Add checking for writable and exectuable pages
From: Mark Rutland @ 2016-09-30  2:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160929213257.30505-4-labbott@redhat.com>

Hi,

On Thu, Sep 29, 2016 at 02:32:57PM -0700, Laura Abbott wrote:
> Page mappings with full RWX permissions are a security risk. x86
> has an option to walk the page tables and dump any bad pages.
> (See e1a58320a38d ("x86/mm: Warn on W^X mappings")). Add a similar
> implementation for arm64.
> 
> Signed-off-by: Laura Abbott <labbott@redhat.com>

> @@ -31,6 +32,8 @@ struct ptdump_info {
>  	const struct addr_marker	*markers;
>  	unsigned long			base_addr;
>  	unsigned long			max_addr;

(unrelated aside: it looks like max_addr is never used or even assigned to;
care to delete it in a prep patch?)

> +	/* Internal, do not touch */
> +	struct list_head		node;
>  };

> +static LIST_HEAD(dump_info);

With the EFI runtime map tables it's unfortunately valid (and very likely with
64K pages) that there will be RWX mappings, at least with contemporary versions
of the UEFI spec. Luckily, those are only installed rarely and transiently.

Given that (and other potential ptdump users), I don't think we should have a
dynamic list of ptdump_infos for W^X checks, and should instead have
ptdump_check_wx() explicitly check the tables we care about. More comments
below on that.

I think we only care about the swapper and hyp tables, as nothing else is
permanent. Does that sound sane?

>  struct prot_bits {
> @@ -219,6 +223,15 @@ static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
>  		unsigned long delta;
>  
>  		if (st->current_prot) {
> +			if (st->check_wx &&
> +			((st->current_prot & PTE_RDONLY) != PTE_RDONLY) &&
> +			((st->current_prot & PTE_PXN) != PTE_PXN)) {
> +				WARN_ONCE(1, "arm64/mm: Found insecure W+X mapping at address %p/%pS\n",
> +					 (void *)st->start_address,
> +					 (void *)st->start_address);
> +				st->wx_pages += (addr - st->start_address) / PAGE_SIZE;
> +			}
> +

Currently note_page() is painful to read due to the indentation and logic.
Rather than adding to that, could we factor this into a helper? e.g.

note_prot_wx(struct pg_state *st, unsigned long addr)
{
	if (!st->check_wx)
		return;
	if ((st->current_prot & PTE_RDONLY) == PTE_RDONLY)
		return;
	if ((st->current_prot & PTE_PXN) == PTE_PXN)
		return;

	WARN_ONCE(1, "arm64/mm: Found insecure W+X mapping at address %p/%pS\n",
		  (void *)st->start_address, (void *)st->start_address);

	st->wx_pages += (addr - st->start_address) / PAGE_SIZE;
}

> +void ptdump_check_wx(void)
> +{
> +	struct ptdump_info *info;
> +
> +	list_for_each_entry(info, &dump_info, node) {
> +		struct pg_state st = {
> +			.seq = NULL,
> +			.marker = info->markers,
> +			.check_wx = true,
> +		};
> +
> +		__walk_pgd(&st, info->mm, info->base_addr);
> +		note_page(&st, 0, 0, 0);
> +		if (st.wx_pages)
> +			pr_info("Checked W+X mappings (%p): FAILED, %lu W+X pages found\n",
> +				info->mm,
> +				st.wx_pages);
> +		else
> +			pr_info("Checked W+X mappings (%p): passed, no W+X pages found\n", info->mm);
> +	}
> +}

As above, I don't think we should use a list of arbitrary ptdump_infos.

Given we won't log addresses in the walking code, I think that we can make up a
trivial marker array, and then just use init_mm direct, e.g. (never even
compile-tested):

void ptdump_check_wx(void)
{
	struct pg_state st = {
		.seq = NULL,
		.marker = (struct addr_markers[]) {
			{ -1, NULL},
		},
		.check_wx = true,
	};

	__walk_pgd(&st, init_mm, 0);
	note_page(&st, 0, 0, 0);
	if (st.wx_pages)
		pr_info("Checked W+X mappings (%p): FAILED, %lu W+X pages found\n",
			info->mm,
			st.wx_pages);
	else
		pr_info("Checked W+X mappings (%p): passed, no W+X pages found\n", info->mm);
}

Otherwise, this looks good to me. Thanks for putting this together!

Mark.

^ permalink raw reply

* [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: CK Hu @ 2016-09-30  3:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473039885-24009-3-git-send-email-hs.liao@mediatek.com>

Hi, HS:

On Mon, 2016-09-05 at 09:44 +0800, HS Liao wrote:
> This patch is first version of Mediatek Command Queue(CMDQ) driver. The
> CMDQ is used to help write registers with critical time limitation,
> such as updating display configuration during the vblank. It controls
> Global Command Engine (GCE) hardware to achieve this requirement.
> Currently, CMDQ only supports display related hardwares, but we expect
> it can be extended to other hardwares for future requirements.
> 
> Signed-off-by: HS Liao <hs.liao@mediatek.com>
> Signed-off-by: CK Hu <ck.hu@mediatek.com>
> ---

[snip...]

> +
> +struct cmdq_task {
> +	struct cmdq		*cmdq;
> +	struct list_head	list_entry;
> +	void			*va_base;
> +	dma_addr_t		pa_base;
> +	size_t			cmd_buf_size; /* command occupied size */
> +	size_t			buf_size; /* real buffer size */
> +	bool			finalized;
> +	struct cmdq_thread	*thread;

I think thread info could be removed from cmdq_task. Only
cmdq_task_handle_error() and cmdq_task_insert_into_thread() use
task->thread and caller of both function has the thread info. So you
could just pass thread info into these two function and remove thread
info in cmdq_task.

> +	struct cmdq_task_cb	cb;

I think this callback function is equal to mailbox client tx_done
callback. It's better to use already-defined interface rather than
creating your own.

> +};
> +

[snip...]

> +
> +static int cmdq_suspend(struct device *dev)
> +{
> +	struct cmdq *cmdq = dev_get_drvdata(dev);
> +	struct cmdq_thread *thread;
> +	int i;
> +	bool task_running = false;
> +
> +	mutex_lock(&cmdq->task_mutex);
> +	cmdq->suspended = true;
> +	mutex_unlock(&cmdq->task_mutex);
> +
> +	for (i = 0; i < ARRAY_SIZE(cmdq->thread); i++) {
> +		thread = &cmdq->thread[i];
> +		if (!list_empty(&thread->task_busy_list)) {
> +			mod_timer(&thread->timeout, jiffies + 1);
> +			task_running = true;
> +		}
> +	}
> +
> +	if (task_running) {
> +		dev_warn(dev, "exist running task(s) in suspend\n");
> +		msleep(20);

Why sleep here? It looks like a recovery but could 20ms recovery
something? I think warning message is enough because you see the warning
message, and you fix the bug, so no need to recovery anything.

> +	}
> +
> +	clk_unprepare(cmdq->clock);
> +	return 0;
> +}
> +

Regards,
CK

^ permalink raw reply

* [PATCH] drm/mediatek: fix a typo
From: Bibby Hsieh @ 2016-09-30  3:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1f324471-bebb-c8bf-75c1-391355c50a99@gmail.com>

On Thu, 2016-09-29 at 10:46 +0200, Matthias Brugger wrote:
> 
> On 29/09/16 06:01, CK Hu wrote:
> > Acked-by: CK Hu <ck.hu@mediatek.com>
> >
> > On Thu, 2016-09-29 at 11:22 +0800, Bibby Hsieh wrote:
> >> Fix the typo: OD_RELAYMODE->OD_CFG
> >>
> 

Hi, Matthias
Thanks for your reply.

> Although it is quite clear what the patch does, could you write one 
> sentence to explain what it does. Maybe explain even which effect it 
> has, which error get fixed etc.

Ok, I will do that.

> As we are getting public available boards now, we should take more care 
> about fixes. If you have a fix for a commit introduced in an earlier 
> version of linux and it should be fixed for this version as well (e.g. 
> v4.6 does have the feature but it does not work correctly) then please 
> add these two lines before your Signed-off-by:
> Fixes: <commit-hash> ("<commit subject line>")
> Cc: stable at vger.kernel.org # v4.6+
> 
> Where v4.6+ stands for the oldest version where this should get fixed.
> 

Ok, but the patch hasn't been merged into v4.8 (just in drm-next [1] and
linux-next [2]), how can I mark that?

[1]
https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next&id=7216436420414144646f5d8343d061355fd23483
[2]
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=7216436420414144646f5d8343d061355fd23483


> Thanks a lot,
> Matthias
> 
> >> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> >> ---
> >>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> >> index df33b3c..aa5f20f 100644
> >> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> >> @@ -123,7 +123,7 @@ static void mtk_od_config(struct mtk_ddp_comp *comp, unsigned int w,
> >>  			  unsigned int bpc)
> >>  {
> >>  	writel(w << 16 | h, comp->regs + DISP_OD_SIZE);
> >> -	writel(OD_RELAYMODE, comp->regs + OD_RELAYMODE);
> >> +	writel(OD_RELAYMODE, comp->regs + OD_CFG);
> >>  	mtk_dither_set(comp, bpc, DISP_OD_CFG);
> >>  }
> >>
> >
> >

-- 
Bibby

^ permalink raw reply

* [PATCH 0/4] Add DMA support for ti_am335x_adc driver
From: John Syne @ 2016-09-30  3:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <ff19f584-e4e5-61ff-046d-68ec37ab51c2@ti.com>

I applied your patches to the following kernel:

github.com/RobertCNelson/bb-kernel.git 
branch am33x-v4.8

Changing 
#define REG_DMAENABLE_CLEAR	0x038
to
#define REG_DMAENABLE_CLEAR	0x03C

Also applying the following DTS changes:

		target = <&tscadc>;
		__overlay__ {

			status = "okay";
			adc {
				ti,adc-channels = <0 1 2 3 4 5 6>;
				ti,chan-step-avg = <0x1 0x1 0x1 0x1 0x1 0x1 0x1>;
				ti,chan-step-opendelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
				ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
			};
		};

Running on a BeagleBoneBlack, I was able to stream samples at 200ksps continuously and it looked stable. 

htop showed CPU utilization between 5% and 7%

Thank you for getting this to work.

Regards,
John




> On Sep 29, 2016, at 6:01 AM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> 
> On Sunday 25 September 2016 03:11 PM, Jonathan Cameron wrote:
>> On 21/09/16 17:11, Mugunthan V N wrote:
>>>> The ADC has a 64 work depth fifo length which holds the ADC data
>>>> till the CPU reads. So when a user program needs a large ADC data
>>>> to operate on, then it has to do multiple reads to get its
>>>> buffer. Currently if the application asks for 4 samples per
>>>> channel with all 8 channels are enabled, kernel can provide only
>>>> 3 samples per channel when all 8 channels are enabled (logs at
>>>> [1]). So with DMA support user can request for large number of
>>>> samples at a time (logs at [2]).
>>>> 
>>>> Tested the patch on AM437x-gp-evm and AM335x Boneblack with the
>>>> patch [3] to enable ADC and pushed a branch for testing [4]
>>>> 
>>>> [1] - http://pastebin.ubuntu.com/23211490/
>>>> [2] - http://pastebin.ubuntu.com/23211492/
>>>> [3] - http://pastebin.ubuntu.com/23211494/
>>>> [4] - git://git.ti.com/~mugunthanvnm/ti-linux-kernel/linux.git iio-dma
>> Just curious.  How fast is the ADC sampling at in these?  Never that
>> obvious for this driver!
>> 
>> I'm also curious as to whether you started to hit the limits of the
>> kfifo based interface.  Might be worth considering adding alternative
>> support for the dma buffers interface which is obviously much lower
>> overhead.
>> 
>> Good to have this work prior to that as the kfifo stuff is somewhat
>> easier to use.
> 
> Currently ADC clock is 3MHz, which can produce a data rate of 225KBps
> per channel with no open delay and no averaging of samples. So when all
> 8 Channels are enables the data rate will be 1.75MBps
> 
> ADC can be operated at 24MHz, which can generate a data rate of 28MBps
> with all 8 channels enabled and no open delay and averaging, but our
> target is to get 800K samples per second per channel which has a data
> rate of 12.5MBps
> 
> I think with this data rate, DMA will be the best option to implement
> without any data loss and less cpu overload to read the ADC samples.
> 
> Regards
> Mugunthan V N
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* ARM juno R2 board USB Issue (EHCI probe failed)
From: Sajjan, Vikas C @ 2016-09-30  4:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <37a463cb-86d5-e0d7-7e8f-50cec3d65c95@arm.com>

Hi Sudeep,

-----Original Message-----
From: Sudeep Holla [mailto:sudeep.holla at arm.com] 
Sent: Tuesday, September 27, 2016 2:38 PM
To: Sajjan, Vikas C <vikas.cha.sajjan@hpe.com>; Vikas Sajjan <sajjan.linux@gmail.com>; linux-usb at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-acpi at vger.kernel.org
Cc: Sudeep Holla <sudeep.holla@arm.com>; mark.rutland at arm.com; lorenzo.pieralisi at arm.com
Subject: Re: ARM juno R2 board USB Issue (EHCI probe failed)



On 27/09/16 09:55, Sajjan, Vikas C wrote:
> Hi Sudeep,
>
> -----Original Message-----
> From: Sudeep Holla [mailto:sudeep.holla at arm.com]
> Sent: Tuesday, September 27, 2016 2:21 PM
> To: Vikas Sajjan <sajjan.linux@gmail.com>; linux-usb at vger.kernel.org; 
> linux-arm-kernel at lists.infradead.org; linux-acpi at vger.kernel.org
> Cc: Sudeep Holla <sudeep.holla@arm.com>; mark.rutland at arm.com; 
> lorenzo.pieralisi at arm.com; Sajjan, Vikas C <vikas.cha.sajjan@hpe.com>
> Subject: Re: ARM juno R2 board USB Issue (EHCI probe failed)
>
> Hi Vikas,
>
> On 27/09/16 09:14, Vikas Sajjan wrote:
>> Adding USB mailing list.
>>
>>
>> On Tue, Sep 27, 2016 at 12:33 PM, Sajjan, Vikas C 
>> <vikas.cha.sajjan@hpe.com> wrote:
>>> Hi All,
>>>
>>> I working on ARM juno R2 board, with latest kernel 4.8.rc7 and I get 
>>> below USB EHCI probe error while booting with acpi=force.
>>>
>
> Are you using the latest UEFI EDK2 ?
> No, I am still using the UEFI binary which came as part of the Juno board.
>
>
>>> [    1.223662] VFIO - User Level meta-driver version: 0.3
>>> [    1.229335] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
>>> [    1.235882] ehci-pci: EHCI PCI platform driver
>>> [    1.240359] ehci-platform: EHCI generic platform driver
>>> [    1.245619] ehci-platform ARMH0D20:00: Error: DMA mask configuration failed
>>> [    1.272491] ehci-platform: probe of ARMH0D20:00 failed with error -5
>>> [    1.278876] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
>>> [    1.285071] ohci-pci: OHCI PCI platform driver
>>> [    1.289548] ohci-platform: OHCI generic platform driver
>>> [    1.294884] usbcore: registered new interface driver usb-storage
>>> [    1.301231] mousedev: PS/2 mouse device common for all mice
>>> [    1.307197] rtc-efi rtc-efi: rtc core: registered rtc-efi as rtc0
>>>
>>> But this error goes off, if I don't force ACPI booting, i.e., if I 
>>> remove acpi=force from kernel command line , USB is detected  and my 
>>> RFS which is in the usb drive, gets mounted successfully.
>>>
>
> As I mentioned in private, I do get the same error if I drop _CCA in 
> USB object of ACPI DSDT. Can you give it a spin with latest UEFI ?
>
> Sure, will try with latest UEFI.
>

I bet that's 8-12 months old. It puts the banner during boot with the build date. You can try to follow [1] or access it from [2]

I was able boot the kernel without any EHCI failure and could get to the shell prompt using the EDK2 in [2].
Thanks for help.

Thanks and Regards
Vikas Sajjan
--
Regards,
Sudeep

[1] https://community.arm.com/docs/DOC-11395
[2]
http://snapshots.linaro.org/member-builds/armlt-platforms-release/32/juno-uefi.zip

^ permalink raw reply

* [PATCH 2/2] mmc: sdhci-of-arasan: mark sdhci_arasan_reset() static
From: Sören Brinkmann @ 2016-09-30  6:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475199459-4775-2-git-send-email-baoyou.xie@linaro.org>

On Fri, 2016-09-30 at 09:37:39 +0800, Baoyou Xie wrote:
> We get 1 warning when building kernel with W=1:
> drivers/mmc/host/sdhci-of-arasan.c:253:6: warning: no previous prototype for 'sdhci_arasan_reset' [-Wmissing-prototypes]
> 
> In fact, this function is only used in the file in which it is
> declared and don't need a declaration, but can be made static.
> So this patch marks it 'static'.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>

There is already a patch for this:
https://patchwork.kernel.org/patch/9349805/

	S?ren

^ permalink raw reply

* [PATCH] drm/mediatek: stop using drm_vblank_count() as the hw frame counter
From: YT Shen @ 2016-09-30  6:45 UTC (permalink / raw)
  To: linux-arm-kernel

using drm_vblank_no_hw_counter() to eliminate kernel warning.

Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index eebb7d8..e286193 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -239,7 +239,7 @@ static struct drm_driver mtk_drm_driver = {
 	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
 			   DRIVER_ATOMIC,
 
-	.get_vblank_counter = drm_vblank_count,
+	.get_vblank_counter = drm_vblank_no_hw_counter,
 	.enable_vblank = mtk_drm_crtc_enable_vblank,
 	.disable_vblank = mtk_drm_crtc_disable_vblank,
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH] pwm: imx: Port "pwm: imx: support output polarity inversion" to Linux v4.7
From: Lukasz Majewski @ 2016-09-30  6:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160911110112.564df4ab@jawa>

Dear all,

> Dear Lothar, Stefan,
> 
> > This patch ports "pwm: imx: support output polarity inversion" patch
> > set written by Lothar Wassmann (v6 from 10.2014).
> > 
> 
> I've read the e-mail from Stefan regarding missing support for pwm-imx
> polarity inversion feature.
> 
> I also would like to see it in ML. Hence, my patch. Lothar, please
> feel free to squash it to your patches when you (I hope :-) ) will
> prepare v7 of this feature.
> 
> I hope that this would help.

Was there any decision about those patches?

Would they be included to main line anytime soon?

Best regards,
?ukasz Majewski

> 
> Best regards,
> ?ukasz Majewski
> 
> > It is used to control backlight of panels via inverted PWM signal.
> > 
> > The "inversion" of PWM output is not an issue at such devices, since
> > separate GPIO pin is responsible for enabling and disabling the
> > panel's backlight.
> > 
> > This patch should be put on top of:
> > 
> > https://patchwork.kernel.org/patch/5065841/
> > https://patchwork.kernel.org/patch/5065821/
> > https://patchwork.kernel.org/patch/5065811/
> > 
> > 
> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > ---
> >  drivers/pwm/pwm-imx.c | 20 ++++++++++++++------
> >  1 file changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > index 471a99e..c37d223 100644
> > --- a/drivers/pwm/pwm-imx.c
> > +++ b/drivers/pwm/pwm-imx.c
> > @@ -181,7 +181,7 @@ static int imx_pwm_config_v2(struct pwm_chip
> > *chip, if (enable)
> >  		cr |= MX3_PWMCR_EN;
> >  
> > -	if (pwm->polarity == PWM_POLARITY_INVERSED)
> > +	if (pwm->args.polarity == PWM_POLARITY_INVERSED)
> >  		cr |= MX3_PWMCR_POUTC;
> >  
> >  	writel(cr, imx->mmio_base + MX3_PWMCR);
> > @@ -201,11 +201,6 @@ static void imx_pwm_set_enable_v2(struct
> > pwm_chip *chip, bool enable) else
> >  		val &= ~MX3_PWMCR_EN;
> >  
> > -	if (chip->pwms[0].polarity == PWM_POLARITY_INVERSED)
> > -		val |= MX3_PWMCR_POUTC;
> > -	else
> > -		val &= ~MX3_PWMCR_POUTC;
> > -
> >  	writel(val, imx->mmio_base + MX3_PWMCR);
> >  }
> >  
> > @@ -253,6 +248,19 @@ static int imx_pwm_set_polarity(struct pwm_chip
> > *chip, struct pwm_device *pwm, enum pwm_polarity polarity)
> >  {
> >  	struct imx_chip *imx = to_imx_chip(chip);
> > +	u32 val;
> > +
> > +	if (polarity == pwm->args.polarity)
> > +		return 0;
> > +
> > +	val = readl(imx->mmio_base + MX3_PWMCR);
> > +
> > +	if (polarity == PWM_POLARITY_INVERSED)
> > +		val |= MX3_PWMCR_POUTC;
> > +	else
> > +		val &= ~MX3_PWMCR_POUTC;
> > +
> > +	writel(val, imx->mmio_base + MX3_PWMCR);
> >  
> >  	dev_dbg(imx->chip.dev, "%s: polarity set to %s\n",
> > __func__, polarity == PWM_POLARITY_INVERSED ? "inverted" :
> > "normal");
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160930/22860f65/attachment.sig>

^ permalink raw reply

* [PATCH v2] arm64: make rpm failed due to incorrect path to Image.gz
From: Vadim Lomovtsev @ 2016-09-30  7:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475160367-2633-1-git-send-email-Vadim.Lomovtsev@caviumnetworks.com>

[Adding Will Deacon]
Sorry, should do this at the very beginning. (

On Thu, Sep 29, 2016 at 07:46:07AM -0700, Vadim Lomovtsev wrote:
> The "make rpm" and "make rpm-pkg" commands for arm64 platform
> are broken due to rpmbuild couldn't find Image.gz file at
> default location (which is kernel src root):
>  cp: cannot stat 'Image.gz': No such file or directory
>  error: Bad exit status from /var/tmp/rpm-tmp.ocFBmP (%install)
> 
> While the correct path to arm64 kernel image file
> is "arch/arm64/boot/Image.gz".
> 
> The exact file name (Image.gz) is stored at KBUILD_IMAGE variable
> and read by rpmbuild with "make image_name" command at
> install phase after kernel build is complete.
> 
> Accordingly to Michal's Marek comment the KBUILD_IMAGE
> variable has to be set to point to actual file.
> 
> Since the KBUILD_IMAGE variable is used in general cases of
> build we need to prevent other build types breakage by changing it.
> 
> The solution is to add to arch/arm64/Makefie extra target "image_name"
> with dependency "KBUILD_IMAGE:=<proper path to Image.gz file>".
> Thus it will allow to set proper path to Image.gz file only for
> the "image_name" build target and this exact value will be picked up
> while rpm build install phase.
> 
> Signed-off-by: Vadim Lomovtsev <Vadim.Lomovtsev@caviumnetworks.com>
> ---
>  arch/arm64/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index ab51aed..09926d3 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -101,6 +101,8 @@ all:	$(KBUILD_IMAGE) $(KBUILD_DTBS)
>  
>  boot := arch/arm64/boot
>  
> +image_name: KBUILD_IMAGE :=$(boot)/$(KBUILD_IMAGE)
> +
>  Image: vmlinux
>  	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
>  
> -- 
> 1.8.3.1
> 

^ permalink raw reply

* [PATCH/RFT 0/4] ARM: shmobile: R-Car Gen2: Allow booting secondary CPU cores in debug mode
From: Magnus Damm @ 2016-09-30  7:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMuHMdUfV46Y_L2yformTPcnGKpbfFawNOrutyeMk5L4rL5BfA@mail.gmail.com>

Hi Geert,

On Tue, Sep 27, 2016 at 9:37 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Simon, Magnus,
>
> On Mon, Aug 22, 2016 at 4:44 PM, Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
>> This patch series is an attempt to allow booting secondary CPU cores on
>> R-Car Gen2 when hardware debug mode is enabled. In this mode, reset
>> requests derived from power-shutoff to the AP-system CPU cores must be
>> enabled before the AP-system cores first resume from power-shutoff. Else
>> resume may fail, causing the system to hang during boot. Currently we
>> avoid the hang by prohibiting booting secondary CPU cores when hardware
>> debug mode is enabled.
>>
>> On all R-Car Gen2 SoCs, hardware debug mode is enabled by setting
>> MD21=1.  On both Koelsch and Lager, this is done by setting mode switch
>> SW8-4 to OFF.
>>
>> Unfortunately the hang is not easy to reproduce: I only saw it (on
>> Koelsch) during real cold boot (power off during the night), and even
>> then it's not guaranteed to trigger. Pressing the reset button
>> afterwards recovers the system, and a subsequent boot will succeed
>> (incl. secondary CPU core boot).
>>
>> This series configures the reset requests as documented in the R-Car
>> Gen2 datasheet, and removes the check for MD21 during secondary CPU
>> bringup.  It was inspired by CPU-specific patches in the BSP by
>> Nakamura-san.
>>
>> This series has been boot-tested on r8a7791/koelsch (both debug mode and
>> normal mode), on r8a7790/lager and r8a7793/gose (normal mode only), and
>> on r8a7794/alt (normal mode UP only).
>
> Any comments?
> Any objection to applying this series?
>
> I've been running my Koelsch with MD21=1 since I posted this series,
> and it has been included in renesas-drivers since the beginning of September.
>
> My main motivation to push this is that it removes two more users of
> rcar_gen2_read_mode_pins(). After this, the only remaining user is the
> clock driver, invoked from rcar_gen2_timer_init().

I have no objections, but I'm curious if the series received enough
testing (with debug mode enabled) on earlier R-Car Gen2 platforms like
r8a7790/lager?

Cheers,

/ magnus

^ permalink raw reply

* [PATCH/RFT 0/4] ARM: shmobile: R-Car Gen2: Allow booting secondary CPU cores in debug mode
From: Geert Uytterhoeven @ 2016-09-30  7:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CANqRtoRf23PQ+HEDEHtkHwoQb325T3ibLztzwAZ81FddoZ=r-Q@mail.gmail.com>

Hi Magnus,

On Fri, Sep 30, 2016 at 9:04 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> On Tue, Sep 27, 2016 at 9:37 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Mon, Aug 22, 2016 at 4:44 PM, Geert Uytterhoeven
>> <geert+renesas@glider.be> wrote:
>>> This patch series is an attempt to allow booting secondary CPU cores on
>>> R-Car Gen2 when hardware debug mode is enabled. In this mode, reset
>>> requests derived from power-shutoff to the AP-system CPU cores must be
>>> enabled before the AP-system cores first resume from power-shutoff. Else
>>> resume may fail, causing the system to hang during boot. Currently we
>>> avoid the hang by prohibiting booting secondary CPU cores when hardware
>>> debug mode is enabled.
>>>
>>> On all R-Car Gen2 SoCs, hardware debug mode is enabled by setting
>>> MD21=1.  On both Koelsch and Lager, this is done by setting mode switch
>>> SW8-4 to OFF.
>>>
>>> Unfortunately the hang is not easy to reproduce: I only saw it (on
>>> Koelsch) during real cold boot (power off during the night), and even
>>> then it's not guaranteed to trigger. Pressing the reset button
>>> afterwards recovers the system, and a subsequent boot will succeed
>>> (incl. secondary CPU core boot).
>>>
>>> This series configures the reset requests as documented in the R-Car
>>> Gen2 datasheet, and removes the check for MD21 during secondary CPU
>>> bringup.  It was inspired by CPU-specific patches in the BSP by
>>> Nakamura-san.
>>>
>>> This series has been boot-tested on r8a7791/koelsch (both debug mode and
>>> normal mode), on r8a7790/lager and r8a7793/gose (normal mode only), and
>>> on r8a7794/alt (normal mode UP only).
>>
>> Any comments?
>> Any objection to applying this series?
>>
>> I've been running my Koelsch with MD21=1 since I posted this series,
>> and it has been included in renesas-drivers since the beginning of September.
>>
>> My main motivation to push this is that it removes two more users of
>> rcar_gen2_read_mode_pins(). After this, the only remaining user is the
>> clock driver, invoked from rcar_gen2_timer_init().
>
> I have no objections, but I'm curious if the series received enough
> testing (with debug mode enabled) on earlier R-Car Gen2 platforms like
> r8a7790/lager?

Let's see what Hiep has to say, who tests Lager with both debug mode
enabled and disabled...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH/RFT 0/4] ARM: shmobile: R-Car Gen2: Allow booting secondary CPU cores in debug mode
From: Magnus Damm @ 2016-09-30  7:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMuHMdWBQcvOaFNCpt8DFHbT1uTkr6Li_seTmnVese_HPKsREA@mail.gmail.com>

Hi Geert,

On Fri, Sep 30, 2016 at 4:09 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Fri, Sep 30, 2016 at 9:04 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> On Tue, Sep 27, 2016 at 9:37 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Mon, Aug 22, 2016 at 4:44 PM, Geert Uytterhoeven
>>> <geert+renesas@glider.be> wrote:
>>>> This patch series is an attempt to allow booting secondary CPU cores on
>>>> R-Car Gen2 when hardware debug mode is enabled. In this mode, reset
>>>> requests derived from power-shutoff to the AP-system CPU cores must be
>>>> enabled before the AP-system cores first resume from power-shutoff. Else
>>>> resume may fail, causing the system to hang during boot. Currently we
>>>> avoid the hang by prohibiting booting secondary CPU cores when hardware
>>>> debug mode is enabled.
>>>>
>>>> On all R-Car Gen2 SoCs, hardware debug mode is enabled by setting
>>>> MD21=1.  On both Koelsch and Lager, this is done by setting mode switch
>>>> SW8-4 to OFF.
>>>>
>>>> Unfortunately the hang is not easy to reproduce: I only saw it (on
>>>> Koelsch) during real cold boot (power off during the night), and even
>>>> then it's not guaranteed to trigger. Pressing the reset button
>>>> afterwards recovers the system, and a subsequent boot will succeed
>>>> (incl. secondary CPU core boot).
>>>>
>>>> This series configures the reset requests as documented in the R-Car
>>>> Gen2 datasheet, and removes the check for MD21 during secondary CPU
>>>> bringup.  It was inspired by CPU-specific patches in the BSP by
>>>> Nakamura-san.
>>>>
>>>> This series has been boot-tested on r8a7791/koelsch (both debug mode and
>>>> normal mode), on r8a7790/lager and r8a7793/gose (normal mode only), and
>>>> on r8a7794/alt (normal mode UP only).
>>>
>>> Any comments?
>>> Any objection to applying this series?
>>>
>>> I've been running my Koelsch with MD21=1 since I posted this series,
>>> and it has been included in renesas-drivers since the beginning of September.
>>>
>>> My main motivation to push this is that it removes two more users of
>>> rcar_gen2_read_mode_pins(). After this, the only remaining user is the
>>> clock driver, invoked from rcar_gen2_timer_init().
>>
>> I have no objections, but I'm curious if the series received enough
>> testing (with debug mode enabled) on earlier R-Car Gen2 platforms like
>> r8a7790/lager?
>
> Let's see what Hiep has to say, who tests Lager with both debug mode
> enabled and disabled...

Good idea! We probably want to know about ES version too.

Cheers,

/ magnus

^ permalink raw reply

* [PATCH] Adding Support for Coresight Components on Zynq 7000.
From: Muhammad Abdul WAHAB @ 2016-09-30  7:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160929143411.GE16583@xsjsorenbubuntu>

Hi S?ren,

Thank you for your remarks. I corrected a few things as you suggested.

 > I'm curious, did you test that with external debug tools. I have the
 > feeling the kernel using the debug HW could interfere with JTAG
 > debuggers, external trace tools, etc.

I did not test with any external debug tools. For testing, I obtained
trace for simple function (e.g. loop) on Xilinx standalone and I then
traced the same function under Linux. Then, I compare both traces
obtained under standalone and under Linux. I decoded the trace to make
  sure that the trace I get corresponds to my function. The instructions
for testing here only allows users to see that coresight components
are enabled and generating traces. The trace still need to be decoded
to make sure whether the trace is correct or not. It can be done by
openCSD[1] library.

 > Use labels please.
I changed the entries to use labels. I don't need phandle anymore.

 >> +        tpiu at F8803000 {
 >>>
 >>> +            compatible = "arm,coresight-tpiu", "arm,primecell";
 >>> +            reg = <0xf8803000 0x1000>;
 >>> +            clocks = <&clkc 47>, <&clkc 16>;
 >>
 >
 > I'm not sure this is correct for every setup. Sorry, that I don't recall
 > all the details, I haven't used tracing in a long time. But I guess this
 > clock is configurable as you're referring an fclk here. The other thing
 > that makes me a little suspicious is, that nothing in here uses the
 > 'dbg_trc' clock that the clock controller provides.

The TPIU setup included in my patch is specific to my configuration of TPIU.
The second clock is configurable but I didn't know how to do so. As in
Vivado setup I chose "fclk1" as the clock for TPIU, I decided to put fclk1.
But, I am not sure about this. I am having some problems when I recover the
trace from TPIU: it is not the same as in ETB even though it resembles a 
lot.
I don't know if the problem is coming from the device tree or from the 
driver.
I will have a look at 'dbg_trc' clock.

 >>
 >> +            clock-names = "apb_pclk", "fclk1";
 >>
 > Those names (at least fclk1) is not a good name for tpiu to identify
 > it's input. fclk1 is a zynq-specific clock, and as mentioned above, it
 > seems likely that this could easily become a different one. The
 > clock-names are meant to identify an input from the consumer's
 > perspective. The correct names should be documented in the DT binding.

The first name was chosen as "apb_pclk" because it was recommended in
Documentation. On the second name, I agree with you but again for TPIU DT
entry, I am not sure how to make this clock configurable. So, that's why
I put the same clock name as I had in my Vivado setup. If you have any
leads on how to make it configurable, I will be happy to take a look
into it.

 >> +            clock-frequency=<0xee6b280>;
 >>
 > I cannot find this property in the binding.
 >

This property was described in clock binding (in an example [2]) and the
value here (250MHz) corresponds to the value I had chosen in Vivado for
TPIU input clock.

 > I think nodes were ordered alphabetically in our DTs.

Yes, I modified the patch to take care of it. Thank you.

Muhammad Abdul WAHAB

[1]: https://github.com/Linaro/OpenCSD
[2]: Documentation/devicetree/bindings/clock/clock-bindings.txt
---
--- linux-4.7/arch/arm/boot/dts/zynq-7000.dtsi.orig    2016-07-24 
21:23:50.000000000 +0200
+++ linux-4.7/arch/arm/boot/dts/zynq-7000.dtsi    2016-09-29 
20:42:44.286322433 +0200
@@ -96,6 +96,57 @@
              rx-fifo-depth = <0x40>;
          };

+        etb at F8801000 {
+            compatible = "arm,coresight-etb10", "arm,primecell";
+            reg = <0xf8801000 0x1000>;
+
+            coresight-default-sink;
+            clocks = <&clkc 47>;
+            clock-names = "apb_pclk";
+
+            port {
+                etb_in_port: endpoint at 0 {
+                    slave-mode;
+                    remote-endpoint = <&replicator_out_port0>;
+                };
+            };
+        };
+
+        funnel at F8804000 {
+            compatible = "arm,coresight-funnel", "arm,primecell";
+            reg = <0xf8804000 0x1000>;
+
+            clocks = <&clkc 47>;
+            clock-names = "apb_pclk";
+            ports {
+                #address-cells = <0x1>;
+                #size-cells = <0x0>;
+
+                port at 0 {
+                    reg = <0x0>;
+                    funnel_out_port0: endpoint {
+                        remote-endpoint = <&replicator_in_port0>;
+                    };
+                };
+
+                port at 1 {
+                    reg = <0x0>;
+                    funnel_in_port0: endpoint {
+                        slave-mode;
+                        remote-endpoint = <&ptm0_out_port>;
+                    };
+                };
+
+                port at 2 {
+                    reg = <0x1>;
+                    funnel_in_port1: endpoint {
+                        slave-mode;
+                        remote-endpoint = <&ptm1_out_port>;
+                    };
+                };
+            };
+        };
+
          gpio0: gpio at e000a000 {
              compatible = "xlnx,zynq-gpio-1.0";
              #gpio-cells = <2>;
@@ -311,6 +362,82 @@
              clocks = <&clkc 4>;
          };

+        ptm0 at F889C000 {
+            compatible = "arm,coresight-etm3x", "arm,primecell";
+            reg = <0xf889c000 0x1000>;
+
+            cpu = <&cpu0>;
+            clocks = <&clkc 47>;
+            clock-names = "apb_pclk";
+
+            port {
+                ptm0_out_port: endpoint {
+                    remote-endpoint = <&funnel_in_port0>;
+                };
+            };
+        };
+
+        ptm1 at F889D000 {
+            compatible = "arm,coresight-etm3x", "arm,primecell";
+            reg = <0xf889d000 0x1000>;
+
+            cpu = <&cpu1>;
+            clocks = <&clkc 47>;
+            clock-names = "apb_pclk";
+
+            port {
+                ptm1_out_port: endpoint {
+                    remote-endpoint = <&funnel_in_port1>;
+                };
+            };
+        };
+
+        replicator {
+            compatible = "arm,coresight-replicator";
+
+            ports {
+                #address-cells = <0x1>;
+                #size-cells = <0x0>;
+
+                port at 0 {
+                    reg = <0x0>;
+                    replicator_out_port0: endpoint {
+                        remote-endpoint = <&etb_in_port>;
+                    };
+                };
+
+                port at 1 {
+                    reg = <0x1>;
+                    replicator_out_port1: endpoint {
+                        remote-endpoint = <&tpiu_in_port>;
+                    };
+                };
+
+                port at 2 {
+                    reg = <0x0>;
+                    replicator_in_port0: endpoint {
+                        slave-mode;
+                        remote-endpoint = <&funnel_out_port0>;
+                    };
+                };
+            };
+        };
+
+        tpiu at F8803000 {
+            compatible = "arm,coresight-tpiu", "arm,primecell";
+            reg = <0xf8803000 0x1000>;
+
+            clocks = <&clkc 47>, <&clkc 46>;
+            clock-names = "apb_pclk", "configurable_clk";
+            clock-frequency=<0xee6b280>;
+
+            port {
+                tpiu_in_port: endpoint at 0 {
+                    slave-mode;
+                    remote-endpoint = <&replicator_out_port0>;
+                };
+            };
+        };
+
          ttc0: timer at f8001000 {
              interrupt-parent = <&intc>;
              interrupts = <0 10 4>, <0 11 4>, <0 12 4>;

^ permalink raw reply

* [PATCH] net: ethernet: mediatek: mark symbols static where possible
From: Baoyou Xie @ 2016-09-30  7:48 UTC (permalink / raw)
  To: linux-arm-kernel

We get 2 warnings when building kernel with W=1:
drivers/net/ethernet/mediatek/mtk_eth_soc.c:2041:5: warning: no previous prototype for 'mtk_get_link_ksettings' [-Wmissing-prototypes]
drivers/net/ethernet/mediatek/mtk_eth_soc.c:2052:5: warning: no previous prototype for 'mtk_set_link_ksettings' [-Wmissing-prototypes]

In fact, these functions are only used in the file in which they are
declared and don't need a declaration, but can be made static.
So this patch marks these functions with 'static'.

Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index ddf20a0..ad4ab97 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2038,8 +2038,8 @@ static int mtk_cleanup(struct mtk_eth *eth)
 	return 0;
 }
 
-int mtk_get_link_ksettings(struct net_device *ndev,
-			   struct ethtool_link_ksettings *cmd)
+static int mtk_get_link_ksettings(struct net_device *ndev,
+				  struct ethtool_link_ksettings *cmd)
 {
 	struct mtk_mac *mac = netdev_priv(ndev);
 
@@ -2049,8 +2049,8 @@ int mtk_get_link_ksettings(struct net_device *ndev,
 	return phy_ethtool_ksettings_get(ndev->phydev, cmd);
 }
 
-int mtk_set_link_ksettings(struct net_device *ndev,
-			   const struct ethtool_link_ksettings *cmd)
+static int mtk_set_link_ksettings(struct net_device *ndev,
+				  const struct ethtool_link_ksettings *cmd)
 {
 	struct mtk_mac *mac = netdev_priv(ndev);
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 3/6] dt/bindings: Add bindings for Tegra GMI controller
From: Jon Hunter @ 2016-09-30  8:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CALw8SCUK0_kvNR_juAc7N_DMQbV=voizqXE6PApc_B_CQHzzOA@mail.gmail.com>

Rob,

On 19/09/16 08:21, Mirza Krak wrote:
> 2016-09-06 12:32 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
>>
>> On 31/08/16 12:22, Mirza Krak wrote:
>>> 2016-08-30 19:06 GMT+02:00 Rob Herring <robh@kernel.org>:
>>
>> ...
>>
>>>>>                  nvidia,snor-cs = <4>;
>>>>
>>>> NAK, no custom CS properties.
>>
>> Ok, so ...
>>
>>> gmi at 70090000 {
>>>         compatible = "nvidia,tegra20-gmi";
>>>         reg = <0x70009000 0x1000>;
>>>         #address-cells = <2>;
>>>         #size-cells = <1>;
>>>         clocks = <&tegra_car TEGRA20_CLK_NOR>;
>>>         clock-names = "gmi";
>>>         resets = <&tegra_car 42>;
>>>         reset-names = "gmi";
>>>         ranges = <4 0 0xd0000000 0xfffffff>;
>>>
>>>         status = "okay";
>>>
>>>         bus at 4,0 {
>>>                 compatible = "simple-bus";
>>>                 #address-cells = <1>;
>>>                 #size-cells = <1>;
>>>                 ranges = <0 4 0 0x40000>;
>>>
>>>                 nvidia,snor-mux-mode;
>>>                 nvidia,snor-adv-inv;
>>>
>>>                 can at 0 {
>>>                         reg = <0 0x100>;
>>>                         ...
>>>                 };
>>>
>>>                 can at 40000 {
>>>                         reg = <0x40000 0x100>;
>>>                         ...
>>>                 };
>>>         };
>>> };
>>>
>>> Have I understood you correct?
>>>
>>> Also wanted to verify the example case where you only have on device
>>> connected to one CS#, from what I see in other implementations it
>>> seems OK to put the CS# in the reg property in that case. Is this
>>> correct?
>>>
>>> Example with one SJA1000 CAN controller connected to the GMI bus
>>> on CS4:
>>>
>>> gmi at 70090000 {
>>>         compatible = "nvidia,tegra20-gmi";
>>>         reg = <0x70009000 0x1000>;
>>>         #address-cells = <2>;
>>>         #size-cells = <1>;
>>>         clocks = <&tegra_car TEGRA20_CLK_NOR>;
>>>         clock-names = "gmi";
>>>         resets = <&tegra_car 42>;
>>>         reset-names = "gmi";
>>>         ranges = <4 0 0xd0000000 0xfffffff>;
>>>
>>>         status = "okay";
>>>
>>>         can at 4,0 {
>>>                 reg = <4 0 0x100>;
>>>                 nvidia,snor-mux-mode;
>>>                 nvidia,snor-adv-inv;
>>>                 ...
>>>         };
>>> };
>>>
>>> Jon, to be able to handle both cases in the driver we would first
>>> attempt to decode the CS# from the ranges property, and fallback to
>>> reg property if no ranges are defined. Does that sound reasonable?
>>
>> Given the above examples that may be supported, is there a
>> better/simpler way to extract the CS# than what Mirza is proposing? For
>> example, from the node-name unit-address?
>>
> 
> Hi.
> 
> I have been on vacation and now I am back and wanted to finalize these
> patch series.
> 
> So pinging this thread to see I we can agree on a solution.
> 
> Rob any comments to my proposal and Jon`s comment?

Can you comment on the above?

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* [PATCH] clk: mvebu: armada-37xx-periph: Fix the clock gate flag
From: Gregory CLEMENT @ 2016-09-30  8:33 UTC (permalink / raw)
  To: linux-arm-kernel

For the gate part of the peripheral clock setting the bit disables the
clock and clearing it enables the clock. This is not the default behavior
of clk_gate component, so we need to use the CLK_GATE_SET_TO_DISABLE flag.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/clk/mvebu/armada-37xx-periph.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
index d5dfbad4ceab..cecb0fdfaef6 100644
--- a/drivers/clk/mvebu/armada-37xx-periph.c
+++ b/drivers/clk/mvebu/armada-37xx-periph.c
@@ -329,6 +329,7 @@ static int armada_3700_add_composite_clk(const struct clk_periph_data *data,
 		gate->lock = lock;
 		gate_ops = gate_hw->init->ops;
 		gate->reg = reg + (u64)gate->reg;
+		gate->flags = CLK_GATE_SET_TO_DISABLE;
 	}
 
 	if (data->rate_hw) {
-- 
2.9.3

^ permalink raw reply related

* [PATCH] ARM: multi_v7_defconfig: enable CONFIG_EFI
From: Jon Hunter @ 2016-09-30  8:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473949700-12814-1-git-send-email-ard.biesheuvel@linaro.org>

Hi Ard,

On 15/09/16 15:28, Ard Biesheuvel wrote:
> This enables CONFIG_EFI for multi_v7_defconfig, which adds support for
> booting via EFI, and for the EFI framebuffer as builtin options. It
> also enables the EFI rtc, the EFI variable pseudo-filesystem and the
> EFI capsule loader as modules.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> 
> We have been happily booting multi_v7_defconfig+CONFIG_EFI=y kernels on
> kernelci for months now, so please consider enabling this by default.
> The increase in compressed kernel footprint is ~30 KB, for the uncompressed
> kernel it's ~10 KB, some of which is .init code.

After this patch was applied the Tegra automated building for
multi_v7_defconfig has been failing on -next [0]. The error I am seeing
is as follows ...

  STUBCPY drivers/firmware/efi/libstub/lib-sort.stub.o
00000000 R_ARM_ABS32       sort
00000004 R_ARM_ABS32       __ksymtab_strings
drivers/firmware/efi/libstub/lib-sort.stub.o: absolute symbol references
not allowed in the EFI stub
make[4]: *** [drivers/firmware/efi/libstub/lib-sort.stub.o] Error 1
make[3]: *** [drivers/firmware/efi/libstub] Error 2
make[2]: *** [drivers/firmware/efi] Error 2
make[1]: *** [drivers/firmware] Error 2
make: *** [drivers] Error 2

The toolchain this builder uses (which is quite old admittedly) is ...

arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.7.3-12ubuntu1) 4.7.3

I am not seeing this error on other machines but for the life of me, I
cannot figure out why this one is failing. Any thoughts?

Cheers
Jon

[0] http://nvtb.github.io/linux-next/		

-- 
nvpublic

^ permalink raw reply

* [PATCH 1/4] mfd: ti_am335x_tscadc: store physical address
From: Mugunthan V N @ 2016-09-30  8:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160927194045.GK21388@dell>

On Wednesday 28 September 2016 01:10 AM, Lee Jones wrote:
> On Wed, 21 Sep 2016, Mugunthan V N wrote:
> 
>> store the physical address of the device in its priv to use it
>> for DMA addressing in the client drivers.
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>>  drivers/mfd/ti_am335x_tscadc.c       | 1 +
>>  include/linux/mfd/ti_am335x_tscadc.h | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
>> index c8f027b..0f3fab4 100644
>> --- a/drivers/mfd/ti_am335x_tscadc.c
>> +++ b/drivers/mfd/ti_am335x_tscadc.c
>> @@ -183,6 +183,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>>  		tscadc->irq = err;
>>  
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	tscadc->tscadc_phys_base = res->start;
> 
> This is unusual.  Can't you use a virt_to_phys() variant instead?
> 

I tried using virt_to_phys(), but its not working for me.
Also saw many drivers uses like this to get physical address
("git grep -n " res->start;" drivers/*").

Regards
Mugunthan V N

^ permalink raw reply

* [PATCH v14 4/4] CMDQ: save more energy in idle
From: Horng-Shyang Liao @ 2016-09-30  8:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474622885.21723.25.camel@mtksdaap41>

On Fri, 2016-09-23 at 17:28 +0800, Horng-Shyang Liao wrote:
> On Thu, 2016-09-22 at 13:22 +0530, Jassi Brar wrote:
> > On Mon, Sep 5, 2016 at 7:14 AM, HS Liao <hs.liao@mediatek.com> wrote:
> > > Use clk_disable_unprepare instead of clk_disable to save more energy
> > > when CMDQ is idle.
> > >
> > > Signed-off-by: HS Liao <hs.liao@mediatek.com>
> > > ---
> > >  drivers/mailbox/mtk-cmdq.c | 54 +++++++++++++++++++++++++++++++++++++++-------
> > 
> > The driver is introduced by second patch of the set, so it makes sense
> > to merge this patch into patch 2/4.
> 
> Hi Jassi,
> 
> Could you take a look at previous discussion between Matthias and me?
> http://lkml.iu.edu/hypermail/linux/kernel/1606.2/05239.html
> His basic idea is to simplify first working version.
> Therefore, I move some code to this patch.
> 
> Thanks,
> HS
> 

Hi Jassi,

What do you think about our previous discussion?

Thanks,
HS

^ permalink raw reply

* [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: Horng-Shyang Liao @ 2016-09-30  8:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474622898.21723.26.camel@mtksdaap41>

On Fri, 2016-09-23 at 17:28 +0800, Horng-Shyang Liao wrote:
> Hi Jassi,
> 
> Please see my inline reply.
> 
> On Thu, 2016-09-22 at 13:47 +0530, Jassi Brar wrote:
> > On Mon, Sep 5, 2016 at 7:14 AM, HS Liao <hs.liao@mediatek.com> wrote:
> [...]
> > > +struct cmdq_base *cmdq_register_device(struct device *dev)
> > > +{
> > > +       struct cmdq_base *cmdq_base;
> > > +       struct resource res;
> > > +       int subsys;
> > > +       u32 base;
> > > +
> > > +       if (of_address_to_resource(dev->of_node, 0, &res))
> > > +               return NULL;
> > > +       base = (u32)res.start;
> > > +
> > > +       subsys = cmdq_subsys_base_to_id(base >> 16);
> > > +       if (subsys < 0)
> > > +               return NULL;
> > > +
> > > +       cmdq_base = devm_kmalloc(dev, sizeof(*cmdq_base), GFP_KERNEL);
> > > +       if (!cmdq_base)
> > > +               return NULL;
> > > +       cmdq_base->subsys = subsys;
> > > +       cmdq_base->base = base;
> > > +
> > > +       return cmdq_base;
> > > +}
> > > +EXPORT_SYMBOL(cmdq_register_device);
> > > +
> > > +struct cmdq_client *cmdq_mbox_create(struct device *dev, int index)
> > > +{
> > > +       struct cmdq_client *client;
> > > +
> > > +       client = kzalloc(sizeof(*client), GFP_KERNEL);
> > > +       client->client.dev = dev;
> > > +       client->client.tx_block = false;
> > > +       client->chan = mbox_request_channel(&client->client, index);
> > > +       return client;
> > > +}
> > > +EXPORT_SYMBOL(cmdq_mbox_create);
> > > +
> > > +int cmdq_task_create(struct device *dev, struct cmdq_task **task_ptr)
> > > +{
> > > +       struct cmdq_task *task;
> > > +       int err;
> > > +
> > > +       task = kzalloc(sizeof(*task), GFP_KERNEL);
> > > +       if (!task)
> > > +               return -ENOMEM;
> > > +       task->cmdq = dev_get_drvdata(dev);
> > > +       err = cmdq_task_realloc_cmd_buffer(task, PAGE_SIZE);
> > > +       if (err < 0) {
> > > +               kfree(task);
> > > +               return err;
> > > +       }
> > > +       *task_ptr = task;
> > > +       return 0;
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_create);
> > > +
> > > +static int cmdq_task_append_command(struct cmdq_task *task, enum cmdq_code code,
> > > +                                   u32 arg_a, u32 arg_b)
> > > +{
> > > +       u64 *cmd_ptr;
> > > +       int err;
> > > +
> > > +       if (WARN_ON(task->finalized))
> > > +               return -EBUSY;
> > > +       if (unlikely(task->cmd_buf_size + CMDQ_INST_SIZE > task->buf_size)) {
> > > +               err = cmdq_task_realloc_cmd_buffer(task, task->buf_size * 2);
> > > +               if (err < 0)
> > > +                       return err;
> > > +       }
> > > +       cmd_ptr = task->va_base + task->cmd_buf_size;
> > > +       (*cmd_ptr) = (u64)((code << CMDQ_OP_CODE_SHIFT) | arg_a) << 32 | arg_b;
> > > +       task->cmd_buf_size += CMDQ_INST_SIZE;
> > > +       return 0;
> > > +}
> > > +
> > > +int cmdq_task_write(struct cmdq_task *task, u32 value, struct cmdq_base *base,
> > > +                   u32 offset)
> > > +{
> > > +       u32 arg_a = ((base->base + offset) & CMDQ_ARG_A_WRITE_MASK) |
> > > +                   (base->subsys << CMDQ_SUBSYS_SHIFT);
> > > +       return cmdq_task_append_command(task, CMDQ_CODE_WRITE, arg_a, value);
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_write);
> > > +
> > > +int cmdq_task_write_mask(struct cmdq_task *task, u32 value,
> > > +                        struct cmdq_base *base, u32 offset, u32 mask)
> > > +{
> > > +       u32 offset_mask = offset;
> > > +       int err;
> > > +
> > > +       if (mask != 0xffffffff) {
> > > +               err = cmdq_task_append_command(task, CMDQ_CODE_MASK, 0, ~mask);
> > > +               if (err < 0)
> > > +                       return err;
> > > +               offset_mask |= CMDQ_WRITE_ENABLE_MASK;
> > > +       }
> > > +       return cmdq_task_write(task, value, base, offset_mask);
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_write_mask);
> > > +
> > > +static const u32 cmdq_event_value[CMDQ_MAX_EVENT] = {
> > > +       /* Display start of frame(SOF) events */
> > > +       [CMDQ_EVENT_DISP_OVL0_SOF] = 11,
> > > +       [CMDQ_EVENT_DISP_OVL1_SOF] = 12,
> > > +       [CMDQ_EVENT_DISP_RDMA0_SOF] = 13,
> > > +       [CMDQ_EVENT_DISP_RDMA1_SOF] = 14,
> > > +       [CMDQ_EVENT_DISP_RDMA2_SOF] = 15,
> > > +       [CMDQ_EVENT_DISP_WDMA0_SOF] = 16,
> > > +       [CMDQ_EVENT_DISP_WDMA1_SOF] = 17,
> > > +       /* Display end of frame(EOF) events */
> > > +       [CMDQ_EVENT_DISP_OVL0_EOF] = 39,
> > > +       [CMDQ_EVENT_DISP_OVL1_EOF] = 40,
> > > +       [CMDQ_EVENT_DISP_RDMA0_EOF] = 41,
> > > +       [CMDQ_EVENT_DISP_RDMA1_EOF] = 42,
> > > +       [CMDQ_EVENT_DISP_RDMA2_EOF] = 43,
> > > +       [CMDQ_EVENT_DISP_WDMA0_EOF] = 44,
> > > +       [CMDQ_EVENT_DISP_WDMA1_EOF] = 45,
> > > +       /* Mutex end of frame(EOF) events */
> > > +       [CMDQ_EVENT_MUTEX0_STREAM_EOF] = 53,
> > > +       [CMDQ_EVENT_MUTEX1_STREAM_EOF] = 54,
> > > +       [CMDQ_EVENT_MUTEX2_STREAM_EOF] = 55,
> > > +       [CMDQ_EVENT_MUTEX3_STREAM_EOF] = 56,
> > > +       [CMDQ_EVENT_MUTEX4_STREAM_EOF] = 57,
> > > +       /* Display underrun events */
> > > +       [CMDQ_EVENT_DISP_RDMA0_UNDERRUN] = 63,
> > > +       [CMDQ_EVENT_DISP_RDMA1_UNDERRUN] = 64,
> > > +       [CMDQ_EVENT_DISP_RDMA2_UNDERRUN] = 65,
> > > +};
> > > +
> > > +int cmdq_task_wfe(struct cmdq_task *task, enum cmdq_event event)
> > > +{
> > > +       u32 arg_b;
> > > +
> > > +       if (event >= CMDQ_MAX_EVENT || event < 0)
> > > +               return -EINVAL;
> > > +
> > > +       /*
> > > +        * WFE arg_b
> > > +        * bit 0-11: wait value
> > > +        * bit 15: 1 - wait, 0 - no wait
> > > +        * bit 16-27: update value
> > > +        * bit 31: 1 - update, 0 - no update
> > > +        */
> > > +       arg_b = CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE;
> > > +       return cmdq_task_append_command(task, CMDQ_CODE_WFE,
> > > +                       cmdq_event_value[event], arg_b);
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_wfe);
> > > +
> > > +int cmdq_task_clear_event(struct cmdq_task *task, enum cmdq_event event)
> > > +{
> > > +       if (event >= CMDQ_MAX_EVENT || event < 0)
> > > +               return -EINVAL;
> > > +
> > > +       return cmdq_task_append_command(task, CMDQ_CODE_WFE,
> > > +                       cmdq_event_value[event], CMDQ_WFE_UPDATE);
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_clear_event);
> > > +
> > > +static int cmdq_task_finalize(struct cmdq_task *task)
> > > +{
> > > +       int err;
> > > +
> > > +       if (task->finalized)
> > > +               return 0;
> > > +
> > > +       /* insert EOC and generate IRQ for each command iteration */
> > > +       err = cmdq_task_append_command(task, CMDQ_CODE_EOC, 0, CMDQ_EOC_IRQ_EN);
> > > +       if (err < 0)
> > > +               return err;
> > > +
> > > +       /* JUMP to end */
> > > +       err = cmdq_task_append_command(task, CMDQ_CODE_JUMP, 0, CMDQ_JUMP_PASS);
> > > +       if (err < 0)
> > > +               return err;
> > > +
> > > +       task->finalized = true;
> > > +       return 0;
> > > +}
> > > +
> > > +int cmdq_task_flush_async(struct cmdq_client *client, struct cmdq_task *task,
> > > +                         cmdq_async_flush_cb cb, void *data)
> > > +{
> > > +       struct cmdq *cmdq = task->cmdq;
> > > +       int err;
> > > +
> > > +       mutex_lock(&cmdq->task_mutex);
> > > +       if (cmdq->suspended) {
> > > +               dev_err(cmdq->mbox.dev, "%s is called after suspended\n",
> > > +                       __func__);
> > > +               mutex_unlock(&cmdq->task_mutex);
> > > +               return -EPERM;
> > > +       }
> > > +
> > > +       err = cmdq_task_finalize(task);
> > > +       if (err < 0) {
> > > +               mutex_unlock(&cmdq->task_mutex);
> > > +               return err;
> > > +       }
> > > +
> > > +       INIT_LIST_HEAD(&task->list_entry);
> > > +       task->cb.cb = cb;
> > > +       task->cb.data = data;
> > > +       task->pa_base = dma_map_single(cmdq->mbox.dev, task->va_base,
> > > +                                      task->cmd_buf_size, DMA_TO_DEVICE);
> > > +
> > > +       mbox_send_message(client->chan, task);
> > > +       /* We can send next task immediately, so just call txdone. */
> > > +       mbox_client_txdone(client->chan, 0);
> > > +       mutex_unlock(&cmdq->task_mutex);
> > > +       return 0;
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_flush_async);
> > > +
> > > +struct cmdq_flush_completion {
> > > +       struct completion cmplt;
> > > +       bool err;
> > > +};
> > > +
> > > +static void cmdq_task_flush_cb(struct cmdq_cb_data data)
> > > +{
> > > +       struct cmdq_flush_completion *cmplt = data.data;
> > > +
> > > +       cmplt->err = data.err;
> > > +       complete(&cmplt->cmplt);
> > > +}
> > > +
> > > +int cmdq_task_flush(struct cmdq_client *client, struct cmdq_task *task)
> > > +{
> > > +       struct cmdq_flush_completion cmplt;
> > > +       int err;
> > > +
> > > +       init_completion(&cmplt.cmplt);
> > > +       err = cmdq_task_flush_async(client, task, cmdq_task_flush_cb, &cmplt);
> > > +       if (err < 0)
> > > +               return err;
> > > +       wait_for_completion(&cmplt.cmplt);
> > > +       return cmplt.err ? -EFAULT : 0;
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_flush);
> > > +
> > > +void cmdq_mbox_free(struct cmdq_client *client)
> > > +{
> > > +       mbox_free_channel(client->chan);
> > > +       kfree(client);
> > > +}
> > > +EXPORT_SYMBOL(cmdq_mbox_free);
> > > +
> > All these exported functions implement the protocol, so should not be
> > a part of this controller driver. That should go into
> > drivers/soc/mediatek/
> > 
> > The controller driver (mtk-cmdq.c) should implement mainly the
> > mbox_chan_ops and mbox.of_xlate.
> > 
> 
> I can do that, but I would like to confirm with Matthias in advance.
> 
> [...]
> > > +       cmdq->irq = irq_of_parse_and_map(node, 0);
> > >
> > why not,  cmdq->irq = platform_get_irq(pdev, 0);
> 
> Will do
> 
> [...]
> > > +static struct platform_driver cmdq_drv = {
> > > +       .probe = cmdq_probe,
> > > +       .remove = cmdq_remove,
> > > +       .driver = {
> > > +               .name = "mtk_cmdq",
> > > +               .owner = THIS_MODULE,
> > >
> > please remove the unnecessary .owner field.
> 
> Will do
> 
> > > +               .pm = &cmdq_pm_ops,
> > > +               .of_match_table = cmdq_of_ids,
> > > +       }
> > > +};
> > > +
> > > +builtin_platform_driver(cmdq_drv);
> > > diff --git a/include/linux/mailbox/mtk-cmdq.h b/include/linux/mailbox/mtk-cmdq.h
> > > new file mode 100644
> > > index 0000000..c3c924d
> > > --- /dev/null
> > > +++ b/include/linux/mailbox/mtk-cmdq.h
> > >
> > The api implemented is Mediateck proprietary, so I think it should be
> > include/linux/soc/mediatek/cmdq.h
> > 
> > 
> > > @@ -0,0 +1,180 @@
> > > +/*
> > > + * Copyright (c) 2015 MediaTek Inc.
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > > + */
> > > +
> > > +#ifndef __MTK_CMDQ_H__
> > > +#define __MTK_CMDQ_H__
> > > +
> > > +#include <linux/mailbox_client.h>
> > > +#include <linux/mailbox_controller.h>
> > >
> > Clients should not need to include mailbox_controller.h
> 
> This is because client needs to know controller's dev.
> 
> Please see my CMDQ v13.
> http://www.spinics.net/lists/kernel/msg2327867.html
> I add mailbox_controller.h for client to get controller's dev,
> so I can remove a node reference in device tree.
> 
> Should I revert the modification of CMDQ v13?


Hi Jassi,

CMDQ clients don't need to know controller device before flush,
and CMDQ driver can get controller device by itself in flushing flow.
So, I think mailbox_controller.h can be removed from here,
and CMDQ v13 doesn't need to be reverted, either.
I will update this part in CMDQ v15.

Thanks,
HS

> > > +#include <linux/platform_device.h>
> > > +#include <linux/types.h>
> > > +
> > > +/* display events in command queue(CMDQ) */
> > > +enum cmdq_event {
> > > +       /* Display start of frame(SOF) events */
> > > +       CMDQ_EVENT_DISP_OVL0_SOF,
> > >
> > you may want to explicitly initialise the first element.
> 
> Will do
> 
> > > +       CMDQ_EVENT_DISP_OVL1_SOF,
> > > +       CMDQ_EVENT_DISP_RDMA0_SOF,
> > > +       CMDQ_EVENT_DISP_RDMA1_SOF,
> > > +       CMDQ_EVENT_DISP_RDMA2_SOF,
> > > +       CMDQ_EVENT_DISP_WDMA0_SOF,
> > > +       CMDQ_EVENT_DISP_WDMA1_SOF,
> > > +       /* Display end of frame(EOF) events */
> > > +       CMDQ_EVENT_DISP_OVL0_EOF,
> > > +       CMDQ_EVENT_DISP_OVL1_EOF,
> > > +       CMDQ_EVENT_DISP_RDMA0_EOF,
> > > +       CMDQ_EVENT_DISP_RDMA1_EOF,
> > > +       CMDQ_EVENT_DISP_RDMA2_EOF,
> > > +       CMDQ_EVENT_DISP_WDMA0_EOF,
> > > +       CMDQ_EVENT_DISP_WDMA1_EOF,
> > > +       /* Mutex end of frame(EOF) events */
> > > +       CMDQ_EVENT_MUTEX0_STREAM_EOF,
> > > +       CMDQ_EVENT_MUTEX1_STREAM_EOF,
> > > +       CMDQ_EVENT_MUTEX2_STREAM_EOF,
> > > +       CMDQ_EVENT_MUTEX3_STREAM_EOF,
> > > +       CMDQ_EVENT_MUTEX4_STREAM_EOF,
> > > +       /* Display underrun events */
> > > +       CMDQ_EVENT_DISP_RDMA0_UNDERRUN,
> > > +       CMDQ_EVENT_DISP_RDMA1_UNDERRUN,
> > > +       CMDQ_EVENT_DISP_RDMA2_UNDERRUN,
> > > +       /* Keep this at the end */
> > > +       CMDQ_MAX_EVENT,
> > > +};
> > > +
> 
> Thanks,
> HS
> 
> 
> Hi Matthias,
> 
> Do you agree with Jassi's comments about moving parts of code back to
> soc/mediatek/ ?
> If I do it, the part in soc/mediatek/ will be similar to a library.
> Could you tell me a good way to handle this situation?
> 
> Thanks,
> HS


Hi Matthias,

Do you have any suggestion about moving parts of code back to
soc/mediatek/ ?

Thanks,
HS

^ permalink raw reply

* [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: Horng-Shyang Liao @ 2016-09-30  8:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475204778.13398.28.camel@mtksdaap41>

Hi CK,

Please see my inline reply.

On Fri, 2016-09-30 at 11:06 +0800, CK Hu wrote:
> Hi, HS:
> 
> On Mon, 2016-09-05 at 09:44 +0800, HS Liao wrote:
> > This patch is first version of Mediatek Command Queue(CMDQ) driver. The
> > CMDQ is used to help write registers with critical time limitation,
> > such as updating display configuration during the vblank. It controls
> > Global Command Engine (GCE) hardware to achieve this requirement.
> > Currently, CMDQ only supports display related hardwares, but we expect
> > it can be extended to other hardwares for future requirements.
> > 
> > Signed-off-by: HS Liao <hs.liao@mediatek.com>
> > Signed-off-by: CK Hu <ck.hu@mediatek.com>
> > ---
> 
> [snip...]
> 
> > +
> > +struct cmdq_task {
> > +	struct cmdq		*cmdq;
> > +	struct list_head	list_entry;
> > +	void			*va_base;
> > +	dma_addr_t		pa_base;
> > +	size_t			cmd_buf_size; /* command occupied size */
> > +	size_t			buf_size; /* real buffer size */
> > +	bool			finalized;
> > +	struct cmdq_thread	*thread;
> 
> I think thread info could be removed from cmdq_task. Only
> cmdq_task_handle_error() and cmdq_task_insert_into_thread() use
> task->thread and caller of both function has the thread info. So you
> could just pass thread info into these two function and remove thread
> info in cmdq_task.

This modification will remove 1 pointer but add 2 pointers. Moreover,
more pointers will need to be delivered between functions for future
extension. IMHO, it would be better to keep thread pointer inside
cmdq_task.

> > +	struct cmdq_task_cb	cb;
> 
> I think this callback function is equal to mailbox client tx_done
> callback. It's better to use already-defined interface rather than
> creating your own.

This is because CMDQ driver allows different callback functions for
different tasks, but mailbox only allows one callback function per
channel. But, I think I can add a wrapper for tx_done to call CMDQ
callback functions. So, I will use tx_done in CMDQ v15.

> > +};
> > +
> 
> [snip...]
> 
> > +
> > +static int cmdq_suspend(struct device *dev)
> > +{
> > +	struct cmdq *cmdq = dev_get_drvdata(dev);
> > +	struct cmdq_thread *thread;
> > +	int i;
> > +	bool task_running = false;
> > +
> > +	mutex_lock(&cmdq->task_mutex);
> > +	cmdq->suspended = true;
> > +	mutex_unlock(&cmdq->task_mutex);
> > +
> > +	for (i = 0; i < ARRAY_SIZE(cmdq->thread); i++) {
> > +		thread = &cmdq->thread[i];
> > +		if (!list_empty(&thread->task_busy_list)) {
> > +			mod_timer(&thread->timeout, jiffies + 1);
> > +			task_running = true;
> > +		}
> > +	}
> > +
> > +	if (task_running) {
> > +		dev_warn(dev, "exist running task(s) in suspend\n");
> > +		msleep(20);
> 
> Why sleep here? It looks like a recovery but could 20ms recovery
> something? I think warning message is enough because you see the warning
> message, and you fix the bug, so no need to recovery anything.

My purpose is context switch to finish timer's work.
I will replace it by schedule().

> > +	}
> > +
> > +	clk_unprepare(cmdq->clock);
> > +	return 0;
> > +}
> > +
> 
> Regards,
> CK

Thanks,
HS

^ permalink raw reply

* [PATCHv4 0/3] Fix hardware handshake on SAM9x5 platforms
From: Richard Genoud @ 2016-09-30  8:57 UTC (permalink / raw)
  To: linux-arm-kernel

Since commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
hardware handshake is enabled"), hardware handshake is not working
anymore on SAM9x5/SAMA5D3/SAM9 platforms.

The first two patches fix the hardware handshake when CTS/RTS pins are
handled by GPIOs.

The last patch fixes hardware handshake when CTS/RTS pins are not GPIOs.

Changes since v3:
 - remove superfuous #include <linux/err.h> (thanks to Uwe)
 - rebase on next-20160930

Changes since v2:
 - remove IS_ERR_OR_NULL() test in patch 1/3 as Uwe suggested.
 - fix typos in patch 2/3
 - rebase on next-20160927
 - simplify the logic in patch 3/3.

Changes since v1:
 - Correct patch 1 with the error found by kbuild.
 - Add Alexandre's Acked-by on patch 2
 - Rewrite patch 3 logic in the light of the on-going discussion
   with Cyrille and Alexandre.

NB: patch 2 NEEDS patch 1 to compile.

Richard Genoud (3):
  serial: mctrl_gpio: implement mctrl_gpio_use_rtscts
  tty/serial: at91: fix hardware handshake with GPIOs
  tty/serial: at91: fix hardware handshake on SAM9x5 (without GPIOs)

 drivers/tty/serial/atmel_serial.c      | 26 +++++++++++++++++---------
 drivers/tty/serial/serial_mctrl_gpio.c |  7 +++++++
 drivers/tty/serial/serial_mctrl_gpio.h | 10 ++++++++++
 3 files changed, 34 insertions(+), 9 deletions(-)

^ permalink raw reply

* [PATCHv4 1/3] serial: mctrl_gpio: implement mctrl_gpio_use_rtscts
From: Richard Genoud @ 2016-09-30  8:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160930085801.21497-1-richard.genoud@gmail.com>

This function returns true if CTS and RTS are used as GPIOs.
Some drivers (like atmel_serial) needs to know if the flow control is
handled by the controller or by GPIOs.

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/tty/serial/serial_mctrl_gpio.c |  7 +++++++
 drivers/tty/serial/serial_mctrl_gpio.h | 10 ++++++++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index d2da6aa7f27d..38e6e784faa2 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -72,6 +72,13 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
 }
 EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
 
+bool mctrl_gpio_use_rtscts(struct mctrl_gpios *gpios)
+{
+	return mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS) &&
+		mctrl_gpio_to_gpiod(gpios, UART_GPIO_RTS);
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_use_rtscts);
+
 unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
 {
 	enum mctrl_gpio_idx i;
diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
index fa000bcff217..c34269733c62 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.h
+++ b/drivers/tty/serial/serial_mctrl_gpio.h
@@ -101,6 +101,11 @@ void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
  */
 void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios);
 
+/*
+ * Return true if both CTS and RTS are used with GPIOs
+ */
+bool mctrl_gpio_use_rtscts(struct mctrl_gpios *gpios);
+
 #else /* GPIOLIB */
 
 static inline
@@ -152,6 +157,11 @@ static inline void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
 {
 }
 
+static inline bool mctrl_gpio_use_rtscts(struct mctrl_gpios *gpios)
+{
+	return false;
+}
+
 #endif /* GPIOLIB */
 
 #endif

^ permalink raw reply related

* [PATCHv4 2/3] tty/serial: at91: fix hardware handshake with GPIOs
From: Richard Genoud @ 2016-09-30  8:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160930085801.21497-1-richard.genoud@gmail.com>

Commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
hardware handshake is enabled") broke the hardware handshake when GPIOs
were used.

Hardware handshake with GPIOs used to work before this commit because
the CRTSCTS flag (termios->c_cflag) was set, but not the
ATMEL_US_USMODE_HWHS flag (controller register) ; so hardware handshake
enabled, but not handled by the controller.

This commit restores this behaviour.

NB: -stable is not Cced because it doesn't cleanly apply on 4.1+
and it will also need previous commit:
"serial: mctrl_gpio: implement mctrl_gpio_use_rtscts"

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Fixes: 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when hardware handshake is enabled")
---
 drivers/tty/serial/atmel_serial.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index fd8aa1f4ba78..b01b68ece35c 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2130,8 +2130,12 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 		atmel_uart_writel(port, ATMEL_US_TTGR,
 				  port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
-	} else if (termios->c_cflag & CRTSCTS) {
-		/* RS232 with hardware handshake (RTS/CTS) */
+	} else if ((termios->c_cflag & CRTSCTS) &&
+		   !mctrl_gpio_use_rtscts(atmel_port->gpios)) {
+		/*
+		 * RS232 with hardware handshake (RTS/CTS)
+		 * handled by the controller.
+		 */
 		if (atmel_use_dma_rx(port) && !atmel_use_fifo(port)) {
 			dev_info(port->dev, "not enabling hardware flow control because DMA is used");
 			termios->c_cflag &= ~CRTSCTS;
@@ -2139,7 +2143,7 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 			mode |= ATMEL_US_USMODE_HWHS;
 		}
 	} else {
-		/* RS232 without hadware handshake */
+		/* RS232 without hardware handshake or controlled by GPIOs */
 		mode |= ATMEL_US_USMODE_NORMAL;
 	}
 

^ permalink raw reply related

* [PATCHv4 3/3] tty/serial: at91: fix hardware handshake on SAM9x5 (without GPIOs)
From: Richard Genoud @ 2016-09-30  8:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160930085801.21497-1-richard.genoud@gmail.com>

Commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
hardware handshake is enabled") broke the hardware handshake on SAM9x5
platforms.

On Atmel platforms, the USART can only handle the handware handshake
(ATMEL_US_USMODE_HWHS) if FIFOs or PDC are used.

Thus, ATMEL_US_USMODE_HWHS mode should only be used in this case.

For SAM9x5, there's no FIFOs nor PDC for the USART, so the mode should
be ATMEL_US_USMODE_NORMAL and the RTS pin should be controlled by the
driver.

NB: -stable is not Cced because it doesn't cleanly apply on 4.1+

Tested on SAM9G35-CM with and without DMA

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Fixes: 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when hardware handshake is enabled")
---
 drivers/tty/serial/atmel_serial.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index b01b68ece35c..4d033e6af44a 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2131,19 +2131,23 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 				  port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
 	} else if ((termios->c_cflag & CRTSCTS) &&
-		   !mctrl_gpio_use_rtscts(atmel_port->gpios)) {
+		   !mctrl_gpio_use_rtscts(atmel_port->gpios) &&
+		   (atmel_use_pdc_rx(port) || atmel_use_fifo(port))) {
 		/*
-		 * RS232 with hardware handshake (RTS/CTS)
-		 * handled by the controller.
+		 * Automatic hardware handshake (RTS/CTS) only work with
+		 * FIFOs or PDC.
+		 * Meaning that on SAM9x5 the controller can't handle
+		 * the hardware handshake (no FIFOs nor PDC on these platforms).
 		 */
-		if (atmel_use_dma_rx(port) && !atmel_use_fifo(port)) {
-			dev_info(port->dev, "not enabling hardware flow control because DMA is used");
-			termios->c_cflag &= ~CRTSCTS;
-		} else {
-			mode |= ATMEL_US_USMODE_HWHS;
-		}
+		mode |= ATMEL_US_USMODE_HWHS;
 	} else {
-		/* RS232 without hardware handshake or controlled by GPIOs */
+		/*
+		 * Other cases are:
+		 * - RS232 without hardware handshake
+		 * - RS232 with hardware handshake and:
+		 *   - controller unable to handle CTS/RTS by itself
+		 *   - or CTS/RTS handled by GPIOs
+		 */
 		mode |= ATMEL_US_USMODE_NORMAL;
 	}
 

^ permalink raw reply related


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