* [GIT PULL 4/4] Broadcom defconfig-arm64 changes for 4.9 (part 2)
From: Florian Fainelli @ 2016-09-30 19:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475263395-27653-1-git-send-email-f.fainelli@gmail.com>
The following changes since commit 29b4817d4018df78086157ea3a55c1d9424a7cfc:
Linux 4.8-rc1 (2016-08-07 18:18:00 -0700)
are available in the git repository at:
http://github.com/Broadcom/stblinux.git tags/arm-soc/for-4.9/defconfig-arm64
for you to fetch changes up to 51e3fb1d3f514cd336faf185df73b25fca194773:
Merge tag 'bcm2835-defconfig-64-next-2016-09-22' into defconfig-arm64/next (2016-09-30 12:02:29 -0700)
----------------------------------------------------------------
This pull request contains Broadcom ARM64-based SoCs defconfig changes for 4.9,
please pull the following changes:
- Eric updates the ARMv8 defconfig to contain everything that is needed to run
a 64-bit kernel on the Raspberry Pi 3
----------------------------------------------------------------
Eric Anholt (1):
arm64: Add BCM2835 (Raspberry Pi 3) support to the defconfig
Florian Fainelli (1):
Merge tag 'bcm2835-defconfig-64-next-2016-09-22' into defconfig-arm64/next
arch/arm64/configs/defconfig | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
^ permalink raw reply
* [PATCH 0/3] Support userspace irqchip with arch timers
From: Alexander Graf @ 2016-09-30 19:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160930154335.GB7996@cbox>
On 30.09.16 17:43, Christoffer Dall wrote:
> On Fri, Sep 30, 2016 at 05:38:11PM +0200, Alexander Graf wrote:
>>
>>
>> On 30.09.16 16:54, Alexander Graf wrote:
>>>
>>>
>>> On 27.09.16 21:08, Christoffer Dall wrote:
>>>> Hi Alex,
>>>>
>>>> Marc and I have been looking at this during Linaro connect and have
>>>> slightly reworked your patch into this small series.
>>>>
>>>> It would be good if you could have a look at it and test it out.
>>>>
>>>> I've tested it with your QEMU, and it works for UP, but secondary CPUs
>>>> fail to come up, and it looks like the kernel never gets an IPI for
>>>> those CPUs from userspace. Any chance you're willing to take a look at
>>>> that?
>>>
>>> I still need to see whether I can come up with a prettier solution, but
>>> for now this works:
>>>
>>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>>
>> Eh, no, not in i386 code :). But the problem seems to be a missing
>> mpstate sync.
>>
> Yeah, that looked really dodgy. Have you tested it? :)
This time around tested with the correct command line parameters I hope
:). I'll send a pretty patch later.
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index b4c8fe2..b549f00 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -173,6 +173,12 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
*/
kvm_async_interrupts_allowed = true;
+ /*
+ * PSCI wakes up secondary cores, so we always need to
+ * have vCPUs waiting in kernel space
+ */
+ kvm_halt_in_kernel_allowed = true;
+
cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
type_register_static(&host_arm_cpu_type_info);
Alex
^ permalink raw reply related
* [RFC] arm64: Enforce observed order for spinlock and data
From: Mark Rutland @ 2016-09-30 19:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475257257-23072-1-git-send-email-bdegraaf@codeaurora.org>
On Fri, Sep 30, 2016 at 01:40:57PM -0400, Brent DeGraaf wrote:
> Prior spinlock code solely used load-acquire and store-release
> semantics to ensure ordering of the spinlock lock and the area it
> protects. However, store-release semantics and ordinary stores do
> not protect against accesses to the protected area being observed
> prior to the access that locks the lock itself.
>
> While the load-acquire and store-release ordering is sufficient
> when the spinlock routines themselves are strictly used, other
> kernel code that references the lock values directly (e.g. lockrefs)
> could observe changes to the area protected by the spinlock prior
> to observance of the lock itself being in a locked state, despite
> the fact that the spinlock logic itself is correct.
If the spinlock logic is correct, why are we changing that, and not the lockref
code that you say has a problem?
What exactly goes wrong in the lockref code? Can you give a concrete example?
Why does the lockref code accesses lock-protected fields without taking the
lock first? Wouldn't concurrent modification be a problem regardless?
> + /*
> + * Yes: The store done on this cpu was the one that locked the lock.
> + * Store-release one-way barrier on LL/SC means that accesses coming
> + * after this could be reordered into the critical section of the
I assume you meant s/store-release/load-acquire/ here. This does not make sense
to me otherwise.
> + * load-acquire/store-release, where we did not own the lock. On LSE,
> + * even the one-way barrier of the store-release semantics is missing,
Likewise (for the LSE case description).
> + * so LSE needs an explicit barrier here as well. Without this, the
> + * changed contents of the area protected by the spinlock could be
> + * observed prior to the lock.
> + */
By whom? We generally expect that if data is protected by a lock, you take the
lock before accessing it. If you expect concurrent lockless readers, then
there's a requirement on the writer side to explicitly provide the ordering it
requires -- spinlocks are not expected to provide that.
So, why aren't those observers taking the lock?
What pattern of accesses are made by readers and writers such that there is a
problem?
What does this result in?
> +" dmb ish\n"
> +" b 3f\n"
> +"4:\n"
> /*
> * No: spin on the owner. Send a local event to avoid missing an
> * unlock before the exclusive load.
> @@ -116,7 +129,15 @@ static inline void arch_spin_lock(arch_spinlock_t *lock)
> " ldaxrh %w2, %4\n"
> " eor %w1, %w2, %w0, lsr #16\n"
> " cbnz %w1, 2b\n"
> - /* We got the lock. Critical section starts here. */
> + /*
> + * We got the lock and have observed the prior owner's store-release.
> + * In this case, the one-way barrier of the prior owner that we
> + * observed combined with the one-way barrier of our load-acquire is
> + * enough to ensure accesses to the protected area coming after this
> + * are not accessed until we own the lock. In this case, other
> + * observers will not see our changes prior to observing the lock
> + * itself. Critical locked section starts here.
> + */
Each of these comments ends up covers, and their repeated presence makes the
code harder to read. If there's a common problem, note it once at the top of
the file.
Thanks,
Mark.
^ permalink raw reply
* [PATCH] drm/sun4i: Check that the plane coordinates are not negative
From: Ville Syrjälä @ 2016-09-30 20:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160930183348.75541582@bbrezillon>
On Fri, Sep 30, 2016 at 06:33:48PM +0200, Boris Brezillon wrote:
> On Fri, 30 Sep 2016 19:22:11 +0300
> Ville Syrj?l? <ville.syrjala@linux.intel.com> wrote:
>
> > On Fri, Sep 30, 2016 at 06:08:26PM +0200, Boris Brezillon wrote:
> > > On Fri, 30 Sep 2016 16:33:20 +0200
> > > Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> > >
> > > > Our planes cannot be set at negative coordinates. Make sure we reject such
> > > > configuration.
> > > >
> > > > Reported-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > > ---
> > > > drivers/gpu/drm/sun4i/sun4i_layer.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
> > > > index f0035bf5efea..f5463c4c2cde 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun4i_layer.c
> > > > +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
> > > > @@ -29,6 +29,9 @@ struct sun4i_plane_desc {
> > > > static int sun4i_backend_layer_atomic_check(struct drm_plane *plane,
> > > > struct drm_plane_state *state)
> > > > {
> > > > + if ((state->crtc_x < 0) || (state->crtc_y < 0))
> > > > + return -EINVAL;
> > > > +
> > >
> > > Hm, I think it's a perfectly valid use case from the DRM framework and
> > > DRM user PoV: you may want to place your plane at a negative CRTC
> > > offset (which means part of the plane will be hidden).
> > >
> > > Maybe I'm wrong, but it seems you can support that by adapting the
> > > start address of your framebuffer pointer and the layer size.
> > >
> > > Have you tried doing something like that?
> >
> > You shouldn't even be looking at the user provided coordinates. Also
> > you can't return an error from the atomic update hook. The void return
> > value should have been a decent hint ;)
>
> Note that Maxime is not returning a value from the atomic update
> implementation (it's done in the atomic_check implem),
Ah, missed that. Anyway it's still the wrong thing to check since
negative coordinates are a perfectly valid thing to have.
> and I'm not
> checking crtc_x,y consistency at all (which is obviously wrong), I'm
> just blindly patching the values in sun4i_backend helpers.
>
> > The right fix would be
> > to move all the error handling into the atomic check hook, which
> > probably should just call the helper to clip the coordinates and
> > whatnot. Then the update hook can just use at the clipped
> > coordinates when programming the hw registers.
>
> That's probably the best approach indeed, but that means having our
> private sun4i_plane_state struct where we would store the patched
> crtc_{w,h,x,y} info.
Nope. The src/dst rects are now stored in drm_plane_state. Assuming my
patches went in already. I think they did.
>
> Anyway, before we do that, that's probably better to check if it really
> works on this HW (which is why I sent this informal patch).
Most hardware can't do negative coordinates natively. But when things
get clipped, the negative coordinates disappear. Then the only question
left is whether the resulting coordinates are fullscreen or not, and
whether the hardware can handle the "not" case.
>
> >
> > >
> > > --->8---
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > > index 3ab560450a82..6b68804f3035 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > > @@ -110,15 +110,30 @@ int sun4i_backend_update_layer_coord(struct sun4i_backend *backend,
> > > {
> > > struct drm_plane_state *state = plane->state;
> > > struct drm_framebuffer *fb = state->fb;
> > > + int crtc_w, crtc_h, crtc_x, crtc_y;
> > >
> > > DRM_DEBUG_DRIVER("Updating layer %d\n", layer);
> > >
> > > + crtc_x = state->crtc_x;
> > > + crtc_y = state->crtc_y;
> > > + crtc_w = state->crtc_w;
> > > + crtc_h = state->crtc_h;
> > > +
> > > + if (crtc_x < 0) {
> > > + crtc_w += crtx_x;
> > > + crtc_x = 0;
> > > + }
> > > +
> > > + if (crtc_y < 0) {
> > > + crtc_h += crtx_y;
> > > + crtc_y = 0;
> > > + }
> > > +
> > > if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> > > DRM_DEBUG_DRIVER("Primary layer, updating global size W: %u H: %u\n",
> > > state->crtc_w, state->crtc_h);
> > > regmap_write(backend->regs, SUN4I_BACKEND_DISSIZE_REG,
> > > - SUN4I_BACKEND_DISSIZE(state->crtc_w,
> > > - state->crtc_h));
> > > + SUN4I_BACKEND_DISSIZE(crtc_w, crtc_h));
> > > }
> > >
> > > /* Set the line width */
> > > @@ -130,15 +145,13 @@ int sun4i_backend_update_layer_coord(struct sun4i_backend *backend,
> > > DRM_DEBUG_DRIVER("Layer size W: %u H: %u\n",
> > > state->crtc_w, state->crtc_h);
> > > regmap_write(backend->regs, SUN4I_BACKEND_LAYSIZE_REG(layer),
> > > - SUN4I_BACKEND_LAYSIZE(state->crtc_w,
> > > - state->crtc_h));
> > > + SUN4I_BACKEND_LAYSIZE(crtc_w, crtc_h));
> > >
> > > /* Set base coordinates */
> > > DRM_DEBUG_DRIVER("Layer coordinates X: %d Y: %d\n",
> > > state->crtc_x, state->crtc_y);
> > > regmap_write(backend->regs, SUN4I_BACKEND_LAYCOOR_REG(layer),
> > > - SUN4I_BACKEND_LAYCOOR(state->crtc_x,
> > > - state->crtc_y));
> > > + SUN4I_BACKEND_LAYCOOR(crtc_x, crtc_y));
> > >
> > > return 0;
> > > }
> > > @@ -198,6 +211,12 @@ int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
> > > paddr += (state->src_x >> 16) * bpp;
> > > paddr += (state->src_y >> 16) * fb->pitches[0];
> > >
> > > + if (state->crtc_x < 0)
> > > + paddr -= bpp * state->crtc_x;
> > > +
> > > + if (state->crtc_y < 0)
> > > + paddr -= fb->pitches[0] * state->crtc_y;
> > > +
> > > DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", &paddr);
> > >
> > > /* Write the 32 lower bits of the address (in bits) */
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
--
Ville Syrj?l?
Intel OTC
^ permalink raw reply
* [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC
From: Bhaskar U @ 2016-09-30 21:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <VI1PR0401MB2638683943A984F9C638F4A88DC10@VI1PR0401MB2638.eurprd04.prod.outlook.com>
>-----Original Message-----
>From: Stuart Yoder
>Sent: Friday, September 30, 2016 7:26 PM
>To: Bhaskar U <bhaskar.upadhaya@nxp.com>; Shawn Guo
><shawnguo@kernel.org>
>Cc: devicetree at vger.kernel.org; oss at buserror.net; Prabhakar Kushwaha
><prabhakar.kushwaha@nxp.com>; linux-devel at gforge.freescale.net; Pratiyush
>Srivastava <pratiyush.srivastava@nxp.com>; linux-arm-
>kernel at lists.infradead.org
>Subject: RE: [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC
>
>> >> +&qspi {
>> >> + num-cs = <2>;
>> >> + bus-num = <0>;
>> >> + status = "disabled";
>> >
>> >Why is it being disabled?
>>
>> Ok, will change like below.
>> status = "okay";
>
>The comment was not "change this to okay". The question is why is this disabled?
>Can you explain why it was disabled? Should it have been disasbled? Is qspi
>working and tested on this board?
The intension of putting the status in disabled state is that the qspi functionality is not tested with the up-streamed kernel.
Yes qspi is working and tested on this board with 4.1 kernel version.
>
>>
>> >
>> >> + fsl,ddr-sampling-point = <4>;
>> >
>> >I do not find the bindings for this property, neither how driver supports it.
>>
>> Yes the QSPI DDR mode is not yet up-streamed, so I will remove this property
>as of now.
>>
>> >
>> >> +
>> >> + qflash0: s25fs512s at 0 {
>> >> + compatible = "spansion,m25p80";
>> >> + #address-cells = <1>;
>> >> + #size-cells = <1>;
>> >> + spi-max-frequency = <20000000>;
>> >> + m25p,fast-read;
>> >> + reg = <0>;
>> >> + };
>> >> +};
>> >> +
>> >> +&i2c0 {
>> >> + status = "okay";
>> >> +
>> >> + codec: sgtl5000 at a {
>> >> + #sound-dai-cells = <0>;
>> >> + compatible = "fsl,sgtl5000";
>> >> + reg = <0xa>;
>> >> + VDDA-supply = <®_1p8v>;
>> >> + VDDIO-supply = <®_1p8v>;
>> >> + clocks = <&sys_mclk>;
>> >> + };
>> >> +};
>> >> +
>> >> +&duart0 {
>> >> + status = "okay";
>> >> +};
>> >> +
>> >> +&esdhc0 {
>> >> + status = "disabled";
>> >
>> >We prefer to disable devices which have board level options by
>> >default in <soc>.dtsi, and enable them per availability in <board>.dts.
>>
>> Ok , will make the status as okay i.e. status = "okay";
>
>Again, the feedback was not "set this to okay". Why was esdhc0 set to "disabled"
>here in the first place? Was there a reason?
>
>The comment is that if there are certain boards where esdhc0 is not available,
>then fsl-ls1012a.dtsi should set this to "disabled" and board .dts files should
>override it.
esdhc0 is not there on this board so shall we mark the status in disabled state ?
>
>> >
>> >> +};
>> >> +
>> >> +&esdhc1 {
>> >> + status = "disabled";
>> >> +};
>> >> +
>> >> +&sai2 {
>> >> + status = "disabled";
>> >> +};
>
>Same comment for the above nodes. The fsl-ls1012a.dtsi should set them to
>disabled and any .dts file should override with "ok" if applicable.
esdhc1 is not there on the board, so shall we keep the status of esdhc1 in disabled state ?
sai2 is working and tested on this board, so shall we put the sai2 status as "okay" ?
Earlier when we kept sai2 status as disabled, by that time sai2 was not tested but now it is working fine.
>
>Stuart
^ permalink raw reply
* [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC
From: Bhaskar U @ 2016-09-30 21:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <VI1PR0401MB2638C032B77C9313C17BBE898DC10@VI1PR0401MB2638.eurprd04.prod.outlook.com>
>-----Original Message-----
>From: Stuart Yoder
>Sent: Friday, September 30, 2016 7:28 PM
>To: Bhaskar U <bhaskar.upadhaya@nxp.com>; devicetree at vger.kernel.org;
>shawnguo at kernel.org
>Cc: oss at buserror.net; linux-arm-kernel at lists.infradead.org; linux-
>devel at gforge.freescale.net; Bhaskar U <bhaskar.upadhaya@nxp.com>;
>Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; Pratiyush Srivastava
><pratiyush.srivastava@nxp.com>
>Subject: RE: [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC
>
>> + dspi0: dspi at 2100000 {
>> + compatible = "fsl,ls1012a-dspi",
>> + "fsl,ls1043a-dspi",
>> + "fsl,ls1021a-v1.0-dspi";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0x0 0x2100000 0x0 0x10000>;
>> + interrupts = <0 64 0x4>;
>> + clock-names = "dspi";
>> + clocks = <&clockgen 4 0>;
>> + spi-num-chipselects = <5>;
>> + big-endian;
>> + status = "enabled";
>
>"enabled" is not a valid status value.
>
So shall I put the status = "okay" ?
But there are other nodes like i2c below having status = "enabled", so do we need to edit them also ? If yes then what should be written in the status ?
i2c0: i2c at 2180000 {
compatible = "fsl,vf610-i2c";
#address-cells = <1>;
#size-cells = <0>;
reg = <0x0 0x2180000 0x0 0x10000>;
interrupts = <0 56 0x4>;
clock-names = "i2c";
clocks = <&clockgen 4 0>;
status = "enabled";
};
i2c1: i2c at 2190000 {
compatible = "fsl,vf610-i2c";
#address-cells = <1>;
#size-cells = <0>;
reg = <0x0 0x2190000 0x0 0x10000>;
interrupts = <0 57 0x4>;
clock-names = "i2c";
clocks = <&clockgen 4 0>;
status = "enabled";
};
>Stuart
^ permalink raw reply
* [linux-devel] [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC
From: Leo Li @ 2016-09-30 21:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AM4PR0401MB2275E76A1EC44FE6DA19D4978CC10@AM4PR0401MB2275.eurprd04.prod.outlook.com>
> -----Original Message-----
> From: linux-devel-bounces at gforge.freescale.net [mailto:linux-devel-
> bounces at gforge.freescale.net] On Behalf Of Bhaskar U
> Sent: Friday, September 30, 2016 4:19 PM
> To: Stuart Yoder <stuart.yoder@nxp.com>; devicetree at vger.kernel.org;
> shawnguo at kernel.org
> Cc: oss at buserror.net; linux-devel at gforge.freescale.net; Pratiyush Srivastava
> <pratiyush.srivastava@nxp.com>; Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com>; linux-arm-kernel at lists.infradead.org
> Subject: Re: [linux-devel] [PATCH v2 1/1] arm64: Add DTS support for FSL's
> LS1012A SoC
>
>
>
> >-----Original Message-----
> >From: Stuart Yoder
> >Sent: Friday, September 30, 2016 7:28 PM
> >To: Bhaskar U <bhaskar.upadhaya@nxp.com>; devicetree at vger.kernel.org;
> >shawnguo at kernel.org
> >Cc: oss at buserror.net; linux-arm-kernel at lists.infradead.org; linux-
> >devel at gforge.freescale.net; Bhaskar U <bhaskar.upadhaya@nxp.com>;
> >Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; Pratiyush Srivastava
> ><pratiyush.srivastava@nxp.com>
> >Subject: RE: [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A
> >SoC
> >
> >> + dspi0: dspi at 2100000 {
> >> + compatible = "fsl,ls1012a-dspi",
> >> + "fsl,ls1043a-dspi",
> >> + "fsl,ls1021a-v1.0-dspi";
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> + reg = <0x0 0x2100000 0x0 0x10000>;
> >> + interrupts = <0 64 0x4>;
> >> + clock-names = "dspi";
> >> + clocks = <&clockgen 4 0>;
> >> + spi-num-chipselects = <5>;
> >> + big-endian;
> >> + status = "enabled";
> >
> >"enabled" is not a valid status value.
> >
>
> So shall I put the status = "okay" ?
>
> But there are other nodes like i2c below having status = "enabled", so do we
> need to edit them also ? If yes then what should be written in the status ?
> i2c0: i2c at 2180000 {
> compatible = "fsl,vf610-i2c";
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <0x0 0x2180000 0x0 0x10000>;
> interrupts = <0 56 0x4>;
> clock-names = "i2c";
> clocks = <&clockgen 4 0>;
> status = "enabled";
> };
>
> i2c1: i2c at 2190000 {
> compatible = "fsl,vf610-i2c";
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <0x0 0x2190000 0x0 0x10000>;
> interrupts = <0 57 0x4>;
> clock-names = "i2c";
> clocks = <&clockgen 4 0>;
> status = "enabled";
> };
The default is "okay". You should just remove these status properties.
Regards,
Leo
^ permalink raw reply
* [linux-devel] [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC
From: Shawn Guo @ 2016-09-30 21:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AM4PR0401MB22757E27E45F67FA5657AAAF8CC10@AM4PR0401MB2275.eurprd04.prod.outlook.com>
On Fri, Sep 30, 2016 at 11:38:29AM +0000, Bhaskar U wrote:
>
>
> >-----Original Message-----
> >From: Scott Wood
> >Sent: Tuesday, September 06, 2016 10:36 PM
> >To: Shawn Guo <shawnguo@kernel.org>; Stuart Yoder <stuart.yoder@nxp.com>;
> >Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>
> >Cc: devicetree at vger.kernel.org; Bhaskar U <bhaskar.upadhaya@nxp.com>; Scott
> >Wood <oss@buserror.net>; Prabhakar Kushwaha
> ><prabhakar.kushwaha@nxp.com>; linux-devel at gforge.freescale.net; Pratiyush
> >Srivastava <pratiyush.srivastava@nxp.com>; linux-arm-
> >kernel at lists.infradead.org
> >Subject: Re: [linux-devel] [PATCH v2 1/1] arm64: Add DTS support for FSL's
> >LS1012A SoC
> >
> >On 09/04/2016 08:47 PM, Shawn Guo wrote:
> >> On Tue, Aug 30, 2016 at 02:07:17PM +0000, Stuart Yoder wrote:
> >>>> On Mon, Aug 29, 2016 at 12:51:01PM -0500, Scott Wood wrote:
> >>>>> On Mon, 2016-08-29 at 17:52 +0800, Shawn Guo wrote:
> >>>>>> On Fri, Aug 26, 2016 at 03:57:21PM +0530, Bhaskar Upadhaya wrote:
> >>>>>>>
> >>>>>>> + clockgen: clocking at 1ee1000 {
> >>>>>>> + compatible = "fsl,ls1012a-clockgen";
> >>>>>> The compatible cannot be found in binding docs.
> >>>>>
> >>>>> From Documentation/devicetree/bindings/clock/qoriq-clock.txt:
> >>>>>
> >>>>> - compatible: Should contain a chip-specific clock block compatible
> >>>>> string and (if applicable) may contain a chassis-version clock
> >>>>> compatible string.
> >>>>>
> >>>>> Chip-specific strings are of the form "fsl,<chip>-clockgen", such as:
> >>>>> * "fsl,p2041-clockgen"
> >>>>> * "fsl,p3041-clockgen"
> >>>>> * "fsl,p4080-clockgen"
> >>>>> * "fsl,p5020-clockgen"
> >>>>> * "fsl,p5040-clockgen"
> >>>>> * "fsl,t4240-clockgen"
> >>>>> * "fsl,b4420-clockgen"
> >>>>> * "fsl,b4860-clockgen"
> >>>>> * "fsl,ls1021a-clockgen"
> >>>>> Chassis-version clock strings include:
> >>>>> * "fsl,qoriq-clockgen-1.0": for chassis 1.0 clocks
> >>>>> * "fsl,qoriq-clockgen-2.0": for chassis 2.0 clocks
> >>>>>
> >>>>> I really hope we don't have to update every single
> >>>>> fsl,<chip>-whatever binding every time a new chip comes out. There
> >>>>> are already other chips not listed, FWIW (e.g. t1040, t2080, ls1043a, and
> >ls2080a). That's why it says "such as".
> >>>>
> >>>> If I remember correctly, DT maintainers want every supported
> >>>> compatible string explicitly listed in bindings doc. And they even
> >>>> added a check into checkpatch.pl with commit bff5da433525
> >>>> ("checkpatch: add DT compatible string documentation checks").
> >>>
> >>> See Documentation/devicetree/bindings/submitting-patches.txt:
> >>>
> >>> 5) The wildcard "<chip>" may be used in compatible strings, as in
> >>> the following example:
> >>>
> >>> - compatible: Must contain '"nvidia,<chip>-pcie",
> >>> "nvidia,tegra20-pcie"' where <chip> is tegra30, tegra132, ...
> >>>
> >>> As in the above example, the known values of "<chip>" should be
> >>> documented if it is used.
> >>>
> >>> It _is_ allowed to use the <chip> wildcard, and so you will not find
> >>> all full compatible strings explicitly listed in bindings. However,
> >>> the chips themselves "should" be listed.
> >>
> >> + Rob and Mark
> >>
> >> Oops, I'm not aware of this DT document. In that case, the DT
> >> document and checkpatch is basically asking for conflicting thing.
> >> Rob, Mark, can you guys please clarify?
> >
> >Checkpatch is a useful tool but it can't get everything right all the time.
> >
>
> So what should we do, shall I add "compatible = "fsl,ls1012a-clockgen";" in Documentation/devicetree/bindings/clock/qoriq-clock.txt ?
I would encourage you to do that.
Shawn
^ permalink raw reply
* [linux-devel] [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC
From: Leo Li @ 2016-09-30 21:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AM4PR0401MB2275F9FBAE954BD420AA67F98CC10@AM4PR0401MB2275.eurprd04.prod.outlook.com>
> -----Original Message-----
> From: linux-devel-bounces at gforge.freescale.net [mailto:linux-devel-
> bounces at gforge.freescale.net] On Behalf Of Bhaskar U
> Sent: Friday, September 30, 2016 4:13 PM
> To: Stuart Yoder <stuart.yoder@nxp.com>; Shawn Guo <shawnguo@kernel.org>
> Cc: devicetree at vger.kernel.org; Pratiyush Srivastava
> <pratiyush.srivastava@nxp.com>; oss at buserror.net; Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com>; linux-devel at gforge.freescale.net; linux-arm-
> kernel at lists.infradead.org
> Subject: Re: [linux-devel] [PATCH v2 1/1] arm64: Add DTS support for FSL's
> LS1012A SoC
>
>
>
> >-----Original Message-----
> >From: Stuart Yoder
> >Sent: Friday, September 30, 2016 7:26 PM
> >To: Bhaskar U <bhaskar.upadhaya@nxp.com>; Shawn Guo
> ><shawnguo@kernel.org>
> >Cc: devicetree at vger.kernel.org; oss at buserror.net; Prabhakar Kushwaha
> ><prabhakar.kushwaha@nxp.com>; linux-devel at gforge.freescale.net;
> >Pratiyush Srivastava <pratiyush.srivastava@nxp.com>; linux-arm-
> >kernel at lists.infradead.org
> >Subject: RE: [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A
> >SoC
> >
> >> >> +&qspi {
> >> >> + num-cs = <2>;
> >> >> + bus-num = <0>;
> >> >> + status = "disabled";
> >> >
> >> >Why is it being disabled?
> >>
> >> Ok, will change like below.
> >> status = "okay";
> >
> >The comment was not "change this to okay". The question is why is this
> disabled?
> >Can you explain why it was disabled? Should it have been disasbled? Is qspi
> >working and tested on this board?
>
> The intension of putting the status in disabled state is that the qspi functionality
> is not tested with the up-streamed kernel.
> Yes qspi is working and tested on this board with 4.1 kernel version.
> >
> >>
> >> >
> >> >> + fsl,ddr-sampling-point = <4>;
> >> >
> >> >I do not find the bindings for this property, neither how driver supports it.
> >>
> >> Yes the QSPI DDR mode is not yet up-streamed, so I will remove this
> >> property
> >as of now.
> >>
> >> >
> >> >> +
> >> >> + qflash0: s25fs512s at 0 {
> >> >> + compatible = "spansion,m25p80";
> >> >> + #address-cells = <1>;
> >> >> + #size-cells = <1>;
> >> >> + spi-max-frequency = <20000000>;
> >> >> + m25p,fast-read;
> >> >> + reg = <0>;
> >> >> + };
> >> >> +};
> >> >> +
> >> >> +&i2c0 {
> >> >> + status = "okay";
> >> >> +
> >> >> + codec: sgtl5000 at a {
> >> >> + #sound-dai-cells = <0>;
> >> >> + compatible = "fsl,sgtl5000";
> >> >> + reg = <0xa>;
> >> >> + VDDA-supply = <®_1p8v>;
> >> >> + VDDIO-supply = <®_1p8v>;
> >> >> + clocks = <&sys_mclk>;
> >> >> + };
> >> >> +};
> >> >> +
> >> >> +&duart0 {
> >> >> + status = "okay";
> >> >> +};
> >> >> +
> >> >> +&esdhc0 {
> >> >> + status = "disabled";
> >> >
> >> >We prefer to disable devices which have board level options by
> >> >default in <soc>.dtsi, and enable them per availability in <board>.dts.
> >>
> >> Ok , will make the status as okay i.e. status = "okay";
> >
> >Again, the feedback was not "set this to okay". Why was esdhc0 set to
> "disabled"
> >here in the first place? Was there a reason?
> >
> >The comment is that if there are certain boards where esdhc0 is not
> >available, then fsl-ls1012a.dtsi should set this to "disabled" and
> >board .dts files should override it.
>
> esdhc0 is not there on this board so shall we mark the status in disabled state ?
>
> >
> >> >
> >> >> +};
> >> >> +
> >> >> +&esdhc1 {
> >> >> + status = "disabled";
> >> >> +};
> >> >> +
> >> >> +&sai2 {
> >> >> + status = "disabled";
> >> >> +};
> >
> >Same comment for the above nodes. The fsl-ls1012a.dtsi should set them
> >to disabled and any .dts file should override with "ok" if applicable.
>
> esdhc1 is not there on the board, so shall we keep the status of esdhc1 in
> disabled state ?
> sai2 is working and tested on this board, so shall we put the sai2 status as
> "okay" ?
> Earlier when we kept sai2 status as disabled, by that time sai2 was not tested but
> now it is working fine.
The convention is to set the status of these optional nodes to "disabled" in the SoC dtsi. And enable the nodes needed in board dts by overwriting the status to "okay". It could be confusing if you use both "disabled" and "okay" in the board dts file.
Regards,
Leo
^ permalink raw reply
* [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC
From: Shawn Guo @ 2016-09-30 21:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AM4PR0401MB2275F9FBAE954BD420AA67F98CC10@AM4PR0401MB2275.eurprd04.prod.outlook.com>
On Fri, Sep 30, 2016 at 09:13:11PM +0000, Bhaskar U wrote:
> >> >> +&qspi {
> >> >> + num-cs = <2>;
> >> >> + bus-num = <0>;
> >> >> + status = "disabled";
> >> >
> >> >Why is it being disabled?
> >>
> >> Ok, will change like below.
> >> status = "okay";
> >
> >The comment was not "change this to okay". The question is why is this disabled?
> >Can you explain why it was disabled? Should it have been disasbled? Is qspi
> >working and tested on this board?
>
> The intension of putting the status in disabled state is that the qspi functionality is not tested with the up-streamed kernel.
> Yes qspi is working and tested on this board with 4.1 kernel version.
Please only add those board level device node after it's been tested on
the board.
> >> >> +&esdhc0 {
> >> >> + status = "disabled";
> >> >
> >> >We prefer to disable devices which have board level options by
> >> >default in <soc>.dtsi, and enable them per availability in <board>.dts.
> >>
> >> Ok , will make the status as okay i.e. status = "okay";
> >
> >Again, the feedback was not "set this to okay". Why was esdhc0 set to "disabled"
> >here in the first place? Was there a reason?
> >
> >The comment is that if there are certain boards where esdhc0 is not available,
> >then fsl-ls1012a.dtsi should set this to "disabled" and board .dts files should
> >override it.
>
> esdhc0 is not there on this board so shall we mark the status in disabled state ?
For device that has pin-out on board, we should mark it disabled in
<soc>.dtsi by default, and enable it in <board>.dts.
> >> >> +&esdhc1 {
> >> >> + status = "disabled";
> >> >> +};
> >> >> +
> >> >> +&sai2 {
> >> >> + status = "disabled";
> >> >> +};
> >
> >Same comment for the above nodes. The fsl-ls1012a.dtsi should set them to
> >disabled and any .dts file should override with "ok" if applicable.
>
> esdhc1 is not there on the board, so shall we keep the status of esdhc1 in disabled state ?
With esdhc1 being marked "disabled" in <soc>.dtsi by default, we need
to do nothing for board that doesn't have the device.
> sai2 is working and tested on this board, so shall we put the sai2 status as "okay" ?
> Earlier when we kept sai2 status as disabled, by that time sai2 was not tested but now it is working fine.
With sai2 being marked "disabled" in <soc>.dtsi by default, you add
board level node with "okay" after you test the device working.
Shawn
^ permalink raw reply
* [PATCH v5 3/3] pci:aer: add support aer interrupt with none MSI/MSI-X/INTx mode
From: Shawn Guo @ 2016-09-30 22:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <VI1PR0401MB17090ED60A826FDCBFB8B33D92F60@VI1PR0401MB1709.eurprd04.prod.outlook.com>
On Wed, Sep 21, 2016 at 06:54:59AM +0000, Po Liu wrote:
> > > > Will the new kernel work with existing/old DTB? I'm trying to
> > > > understand the dependency between driver and DTS changes.
> > >
> > > Yes, We've never use name 'intr' before. So we remove it is ok.
> > > 'aer' is a dts name for researching it's true interrupt number by
> > kernel. This patch is first time to use name 'aer'. So it must be
> > compatible with existing/old DTB.
> >
> > Does that mean driver and DTS changes can go through separate trees, i.e.
> > PCI and arm-soc, without introducing regressions on either tree?
> > Or does the patch series needs to go in as a whole?
>
> Should be as a whole. The driver base on the dts. Or else, the driver would not found the 'aer' point.
Let me try to understand this one more time. So if the new kernel boots
with an existing DTB, the driver will fail to find 'aer' interrupt,
right? My question is that in this case, the driver will just stop
working or keep working in the same way as old kernel. The former case
is a regression which breaks old DTB support, and the latter is pretty
much like that we are adding a new support, and the DTS change can go
independently with driver part through different subsystem tree.
Hope I make my question clear this time.
Shawn
^ permalink raw reply
* [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC
From: Stuart Yoder @ 2016-09-30 22:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AM4PR0401MB2275F9FBAE954BD420AA67F98CC10@AM4PR0401MB2275.eurprd04.prod.outlook.com>
> -----Original Message-----
> From: Bhaskar U
> Sent: Friday, September 30, 2016 4:13 PM
> To: Stuart Yoder <stuart.yoder@nxp.com>; Shawn Guo <shawnguo@kernel.org>
> Cc: devicetree at vger.kernel.org; oss at buserror.net; Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>;
> linux-devel at gforge.freescale.net; Pratiyush Srivastava <pratiyush.srivastava@nxp.com>; linux-arm-
> kernel at lists.infradead.org
> Subject: RE: [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC
>
>
>
> >-----Original Message-----
> >From: Stuart Yoder
> >Sent: Friday, September 30, 2016 7:26 PM
> >To: Bhaskar U <bhaskar.upadhaya@nxp.com>; Shawn Guo
> ><shawnguo@kernel.org>
> >Cc: devicetree at vger.kernel.org; oss at buserror.net; Prabhakar Kushwaha
> ><prabhakar.kushwaha@nxp.com>; linux-devel at gforge.freescale.net; Pratiyush
> >Srivastava <pratiyush.srivastava@nxp.com>; linux-arm-
> >kernel at lists.infradead.org
> >Subject: RE: [PATCH v2 1/1] arm64: Add DTS support for FSL's LS1012A SoC
> >
> >> >> +&qspi {
> >> >> + num-cs = <2>;
> >> >> + bus-num = <0>;
> >> >> + status = "disabled";
> >> >
> >> >Why is it being disabled?
> >>
> >> Ok, will change like below.
> >> status = "okay";
> >
> >The comment was not "change this to okay". The question is why is this disabled?
> >Can you explain why it was disabled? Should it have been disasbled? Is qspi
> >working and tested on this board?
>
> The intension of putting the status in disabled state is that the qspi functionality is not tested with
> the up-streamed kernel.
> Yes qspi is working and tested on this board with 4.1 kernel version.
If the qspi is supported and you have a high degree of confidence that the
node is correct based on testing on 4.1, I would say keep the node and
mark it as "ok".
> >>
> >> >
> >> >> + fsl,ddr-sampling-point = <4>;
> >> >
> >> >I do not find the bindings for this property, neither how driver supports it.
> >>
> >> Yes the QSPI DDR mode is not yet up-streamed, so I will remove this property
> >as of now.
> >>
> >> >
> >> >> +
> >> >> + qflash0: s25fs512s at 0 {
> >> >> + compatible = "spansion,m25p80";
> >> >> + #address-cells = <1>;
> >> >> + #size-cells = <1>;
> >> >> + spi-max-frequency = <20000000>;
> >> >> + m25p,fast-read;
> >> >> + reg = <0>;
> >> >> + };
> >> >> +};
> >> >> +
> >> >> +&i2c0 {
> >> >> + status = "okay";
> >> >> +
> >> >> + codec: sgtl5000 at a {
> >> >> + #sound-dai-cells = <0>;
> >> >> + compatible = "fsl,sgtl5000";
> >> >> + reg = <0xa>;
> >> >> + VDDA-supply = <®_1p8v>;
> >> >> + VDDIO-supply = <®_1p8v>;
> >> >> + clocks = <&sys_mclk>;
> >> >> + };
> >> >> +};
> >> >> +
> >> >> +&duart0 {
> >> >> + status = "okay";
> >> >> +};
> >> >> +
> >> >> +&esdhc0 {
> >> >> + status = "disabled";
> >> >
> >> >We prefer to disable devices which have board level options by
> >> >default in <soc>.dtsi, and enable them per availability in <board>.dts.
> >>
> >> Ok , will make the status as okay i.e. status = "okay";
> >
> >Again, the feedback was not "set this to okay". Why was esdhc0 set to "disabled"
> >here in the first place? Was there a reason?
> >
> >The comment is that if there are certain boards where esdhc0 is not available,
> >then fsl-ls1012a.dtsi should set this to "disabled" and board .dts files should
> >override it.
>
> esdhc0 is not there on this board so shall we mark the status in disabled state ?
If it's not there then it should be disabled, but it should be set to disabled
in the dtsi file (not the dts).
> >
> >> >
> >> >> +};
> >> >> +
> >> >> +&esdhc1 {
> >> >> + status = "disabled";
> >> >> +};
> >> >> +
> >> >> +&sai2 {
> >> >> + status = "disabled";
> >> >> +};
> >
> >Same comment for the above nodes. The fsl-ls1012a.dtsi should set them to
> >disabled and any .dts file should override with "ok" if applicable.
>
> esdhc1 is not there on the board, so shall we keep the status of esdhc1 in disabled state ?
If it's not there on the board, then yes, mark as disabled.
> sai2 is working and tested on this board, so shall we put the sai2 status as "okay" ?
> Earlier when we kept sai2 status as disabled, by that time sai2 was not tested but now it is working
> fine.
If the board supports it and it's been tested, then yes mark as "okay".
Stuart
^ permalink raw reply
* [PATCH] efi/arm: fix absolute relocation detection for older toolchains
From: Ard Biesheuvel @ 2016-09-30 23:01 UTC (permalink / raw)
To: linux-arm-kernel
When building the ARM kernel with CONFIG_EFI=y, the following build
error may occur when using a less recent version of binutils (2.23 or
older):
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
(and when building with debug symbols, the list above is much longer, and
contains all the internal references between the .debug sections and the
actual code)
This issue is caused by the fact that objcopy v2.23 or earlier does not
support wildcards in its -R and -j options, which means the following
line from the Makefile:
STUBCOPY_FLAGS-y := -R .debug* -R *ksymtab* -R *kcrctab*
fails to take effect, leaving harmless absolute relocations in the binary
that are indistinguishable from relocations that may cause crashes at
runtime due to the fact that these relocations are resolved at link time
using the virtual address of the kernel, which is always different from
the address at which the EFI firmware loads and invokes the stub.
So, as a workaround, disable debug symbols explicitly when building the
stub for ARM, and strip the ksymtab and kcrctab symbols for the only
exported symbol we currently reuse in the stub, which is 'sort'.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
This is a workaround for now. We can revisit this when a need arises to copy
more kernel code into the stub, by which time we could put in a more elaborate
fix, or decide to no longer care about 'older' versions of objcopy.
Since this fixes an ARM specific issue and only affects ARM specific Makefile
variables, I am happy for this to go on top of the arm-soc patch that enables
CONFIG_EFI for ARM's multi_v7_defconfig (queued for v4.9), given that we have
no other changes queued in linux-efi that should conflict with this patch.
Matt, any concerns?
drivers/firmware/efi/libstub/Makefile | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index c06945160a41..5e23e2d305e7 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -11,7 +11,7 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ $(LINUX_INCLUDE) -O2 \
-mno-mmx -mno-sse
cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS))
-cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \
+cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) -g0 \
-fno-builtin -fpic -mno-single-pic-base
cflags-$(CONFIG_EFI_ARMSTUB) += -I$(srctree)/scripts/dtc/libfdt
@@ -79,5 +79,6 @@ quiet_cmd_stubcopy = STUBCPY $@
# decompressor. So move our .data to .data.efistub, which is preserved
# explicitly by the decompressor linker script.
#
-STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
+STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub \
+ -R ___ksymtab+sort -R ___kcrctab+sort
STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/2] drm: zte: add initial vou drm driver
From: Shawn Guo @ 2016-10-01 0:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACvgo51feNJYrHaKT+M06Z7kOUqzYo1Uv_TAHdLhNXPtRUwsvw@mail.gmail.com>
Hi Emil,
On Fri, Sep 30, 2016 at 01:34:14PM +0100, Emil Velikov wrote:
> Hi Shawn,
>
> A couple of fly-by suggestions, which I hope you'll find useful :-)
Thanks for the suggestions.
> On 24 September 2016 at 15:26, Shawn Guo <shawn.guo@linaro.org> wrote:
>
> > +
> > + val = ((vm.hsync_len - 1) << SYNC_WIDE_SHIFT) & SYNC_WIDE_MASK;
> To save some writing/minimise the chances to typos getting, in you can
> have single/collection to static inline functions similar to msm [1].
> On a similar note inline wrappers zte_read/write/mask (around
> writel/readl) will provide quite useful for debugging/tracing :-)
>
> [1] drivers/gpu/drm/msm/adreno/a4xx.xml.h
I would not add a header file with hundreds or thousands of defines
while only tens of them are actually used. For debugging, I prefer
to print particular registers than every single read/write, which
might not be easy to check what I want to check.
> > + if (IS_ERR(zcrtc->pixclk))
> > + return ERR_PTR(PTR_ERR(zcrtc->pixclk));
> You might want to s/ERR_PTR(PTR_ERR// or s/ERR_PTR(PTR_ERR/ERR_CAST/
> through the patch.
Aha, yes.
> > +static int zx_drm_bind(struct device *dev)
> > +{
> > + struct drm_device *drm;
> > + struct zx_drm_private *priv;
> > + int ret;
> > +
> > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + drm = drm_dev_alloc(&zx_drm_driver, dev);
> > + if (!drm)
> > + return -ENOMEM;
> > +
> > + drm->dev_private = priv;
> > + dev_set_drvdata(dev, drm);
> > +
> > + drm_mode_config_init(drm);
> > + drm->mode_config.min_width = 16;
> > + drm->mode_config.min_height = 16;
> > + drm->mode_config.max_width = 4096;
> > + drm->mode_config.max_height = 4096;
> > + drm->mode_config.funcs = &zx_drm_mode_config_funcs;
> > +
> > + ret = drm_dev_register(drm, 0);
> The documentation states that drm_dev_register() should be called
> after the hardware is setup. which some drivers seems to interpret as
> ...
>
> > + if (ret)
> > + goto out_free;
> > +
> > + ret = component_bind_all(dev, drm);
> > + if (ret) {
> > + DRM_ERROR("Failed to bind all components\n");
> > + goto out_unregister;
> > + }
> > +
> > + ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
> > + if (ret < 0) {
> > + DRM_ERROR("failed to initialise vblank\n");
> > + goto out_unbind;
> > + }
> > +
> > + /*
> > + * We will manage irq handler on our own. In this case, irq_enabled
> > + * need to be true for using vblank core support.
> > + */
> > + drm->irq_enabled = true;
> > +
> > + drm_mode_config_reset(drm);
> > + drm_kms_helper_poll_init(drm);
> > +
> > + priv->fbdev = drm_fbdev_cma_init(drm, 32, drm->mode_config.num_crtc,
> > + drm->mode_config.num_connector);
> ... calling it after these. If that's the correct case - perhaps we
> can throw a WARN or similar within the above functions to catch
> present/future misuse ?
Yes. Daniel also pointed it out in his review of the patch.
> > + if (IS_ERR(priv->fbdev)) {
> > + ret = PTR_ERR(priv->fbdev);
> > + priv->fbdev = NULL;
> > + goto out_fini;
> > + }
> > +
> > + return 0;
> > +
> > +out_fini:
> > + drm_kms_helper_poll_fini(drm);
> > + drm_mode_config_cleanup(drm);
> > + drm_vblank_cleanup(drm);
> > +out_unbind:
> > + component_unbind_all(dev, drm);
> > +out_unregister:
> > + drm_dev_unregister(drm);
> > +out_free:
> > + dev_set_drvdata(dev, NULL);
> > + drm_dev_unref(drm);
> > + return ret;
> > +}
> > +
> > +static void zx_drm_unbind(struct device *dev)
> > +{
> > + struct drm_device *drm = dev_get_drvdata(dev);
> > + struct zx_drm_private *priv = drm->dev_private;
> > +
> > + if (priv->fbdev) {
> > + drm_fbdev_cma_fini(priv->fbdev);
> > + priv->fbdev = NULL;
> > + }
> > + drm_kms_helper_poll_fini(drm);
> > + component_unbind_all(dev, drm);
> > + drm_vblank_cleanup(drm);
> > + drm_mode_config_cleanup(drm);
> > + drm_dev_unregister(drm);
> > + drm_dev_unref(drm);
> > + drm->dev_private = NULL;
> > + dev_set_drvdata(dev, NULL);
> This and the teardown path in bind() are asymmetrical. Furthermore you
> want to call drm_dev_unregister() first, according to its
> documentation.
> As mentioned above - perhaps it's worth providing feedback for drivers
> which get the order wrong ?
>
>
>
> > +static int zx_hdmi_bind(struct device *dev, struct device *master, void *data)
> > +{
>
>
> > +
> > + clk_prepare_enable(hdmi->cec_clk);
> > + clk_prepare_enable(hdmi->osc_clk);
> > + clk_prepare_enable(hdmi->xclk);
> > +
> > + ret = zx_hdmi_register(drm, hdmi);
> > + if (ret)
> > + return ret;
> > +
>
> > + return 0;
> > +}
> > +
> > +static void zx_hdmi_unbind(struct device *dev, struct device *master,
> > + void *data)
> > +{
> > + struct zx_hdmi *hdmi = dev_get_drvdata(dev);
> > +
> > + clk_disable_unprepare(hdmi->cec_clk);
> > + clk_disable_unprepare(hdmi->osc_clk);
> > + clk_disable_unprepare(hdmi->xclk);
> Nit: you want the teardown to happen in reverse order of the setup. I
> might have missed a few similar cases within the patch, so please
> double-check.
Okay, I will give it a check through the patch.
> > +static int zx_gl_get_fmt(uint32_t format)
> > +{
> > + switch (format) {
> > + case DRM_FORMAT_ARGB8888:
> > + case DRM_FORMAT_XRGB8888:
> > + return GL_FMT_ARGB8888;
> > + case DRM_FORMAT_RGB888:
> > + return GL_FMT_RGB888;
> > + case DRM_FORMAT_RGB565:
> > + return GL_FMT_RGB565;
> > + case DRM_FORMAT_ARGB1555:
> > + return GL_FMT_ARGB1555;
> > + case DRM_FORMAT_ARGB4444:
> > + return GL_FMT_ARGB4444;
> > + default:
> > + WARN_ONCE(1, "invalid pixel format %d\n", format);
> > + return -EINVAL;
> Afaics the only user of this is atomic_update() and that function
> cannot fail. You might want to move this to the _check() function.
> Same logic goes for the rest of the driver, in case I've missed any.
The function does the conversion from DRM format values to the ones that
hardware accepts. And I need to set up hardware register with the
converted value.
I suppose that the error case in 'default' should never be hit, since
all valid format have been reported to DRM core by gl_formats? In that
case, I can simply drop the WARN and return a sane default format
instead?
Shawn
^ permalink raw reply
* [RFC 3/4] arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support
From: Jeremy McNicoll @ 2016-10-01 0:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160921011214.GF5431@hector.attlocal.net>
On 2016-09-20 6:12 PM, Andy Gross wrote:
> On Thu, Jul 07, 2016 at 05:41:06PM -0700, Jeremy McNicoll wrote:
>> From: Bastian K?cher <mail@kchr.de>
>>
>> Initial device tree support for Qualcomm MSM8994 SoC and
>> Huawei Angler / Google Nexus 6P support.
>>
>> The device tree and the angler_defconfig are based on the
>> device tree from the Google 3.10 kernel tree.
>>
>> The device can be booted into the initrd with only one CPU running.
>>
>> Signed-off-by: Bastian K?cher <mail@kchr.de>
>> Signed-off-by: Jeremy McNicoll <jeremymc@redhat.com>
>> ---
>> arch/arm64/Kconfig.platforms | 13 +
>> arch/arm64/boot/dts/Makefile | 1 +
>> arch/arm64/boot/dts/huawei/Makefile | 5 +
>> .../boot/dts/huawei/msm8994-angler-rev-101.dts | 41 ++
>
> There is nothing to warrant having huawei have their own directory unless they
> are making their own SOC.
>
done
>> arch/arm64/boot/dts/qcom/msm8994-pins.dtsi | 38 ++
>> arch/arm64/boot/dts/qcom/msm8994-v2.0.dtsi | 31 +
>> arch/arm64/boot/dts/qcom/msm8994.dtsi | 237 ++++++++
>> arch/arm64/configs/angler_defconfig | 666 +++++++++++++++++++++
>
> We don't need to add defconfigs. We just need to turn on the options in the
> main defconfig.
>
ok, removed
> Also, when you do have defconfigs, please separate those changes into a separate
> patch.
>
will do.
>> 8 files changed, 1032 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/huawei/Makefile
>> create mode 100644 arch/arm64/boot/dts/huawei/msm8994-angler-rev-101.dts
>> create mode 100644 arch/arm64/boot/dts/qcom/msm8994-pins.dtsi
>> create mode 100644 arch/arm64/boot/dts/qcom/msm8994-v2.0.dtsi
>> create mode 100644 arch/arm64/boot/dts/qcom/msm8994.dtsi
>> create mode 100644 arch/arm64/configs/angler_defconfig
>>
>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>> index 515e669..f253f60d 100644
>> --- a/arch/arm64/Kconfig.platforms
>> +++ b/arch/arm64/Kconfig.platforms
>> @@ -98,6 +98,19 @@ config MACH_LGE
>> help
>> This enables support for the LGE Nexus 5X - BullHead MSM8992.
>>
>> +config ARCH_MSM8994
>> + bool "Qualcomm MSM8994"
>> + depends on ARCH_QCOM
>> + select ARCH_REQUIRE_GPIOLIB
>> + help
>> + This enables support for the Qualcomm MSM8994
>> +
>> +config MACH_HUAWEI
>> + bool "Huawei Angler (MSM8994)"
>> + depends on ARCH_QCOM
>> + help
>> + This enables support for the Huawei Nexus 6P - Angler MSM8994.
>
> Remove both of these. We aren't adding more ARCHs or machs.
>
done
>
>> +
>> config ARCH_ROCKCHIP
>> bool "Rockchip Platforms"
>> select ARCH_HAS_RESET_CONTROLLER
>> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
>> index bde90fb..d199f8b 100644
>> --- a/arch/arm64/boot/dts/Makefile
>> +++ b/arch/arm64/boot/dts/Makefile
>> @@ -9,6 +9,7 @@ dts-dirs += cavium
>> dts-dirs += exynos
>> dts-dirs += freescale
>> dts-dirs += hisilicon
>> +dts-dirs += huawei
>> dts-dirs += marvell
>> dts-dirs += mediatek
>> dts-dirs += nvidia
>> diff --git a/arch/arm64/boot/dts/huawei/Makefile b/arch/arm64/boot/dts/huawei/Makefile
>> new file mode 100644
>> index 0000000..4b31ff4
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/huawei/Makefile
>> @@ -0,0 +1,5 @@
>> +dtb-$(CONFIG_MACH_HUAWEI) += msm8994-angler-rev-101.dtb
>> +
>> +always := $(dtb-y)
>> +subdir-y := $(dts-dirs)
>> +clean-files := *.dtb
>> diff --git a/arch/arm64/boot/dts/huawei/msm8994-angler-rev-101.dts b/arch/arm64/boot/dts/huawei/msm8994-angler-rev-101.dts
>> new file mode 100644
>> index 0000000..07a71d6
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/huawei/msm8994-angler-rev-101.dts
>> @@ -0,0 +1,41 @@
>> +/* Copyright (c) 2015, Huawei Inc. All rights reserved.
>> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only 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.
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "../qcom/msm8994-v2.0.dtsi"
>> +
>> +/ {
>> + model = "HUAWEI MSM8994 ANGLER rev-1.01";
>> + compatible = "qcom,msm8994";
>> + qcom,board-id= <8026 0>;
>> +};
>> +
>> +/ {
>> + aliases {
>> + serial0 = &blsp1_uart2;
>> + };
>> +
>> + chosen {
>> + stdout-path = "serial0";
>> + };
>> +
>> + soc {
>> + serial at f991e000 {
>> + status = "okay";
>> + pinctrl-names = "default", "sleep";
>> + pinctrl-0 = <&blsp1_uart2_default>;
>> + pinctrl-1 = <&blsp1_uart2_sleep>;
>> + };
>> + };
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/msm8994-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8994-pins.dtsi
>> new file mode 100644
>> index 0000000..0e4eea0
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/msm8994-pins.dtsi
>> @@ -0,0 +1,38 @@
>> +/*
>> + * Copyright (c) 2013-2016, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only 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.
>> + */
>> +
>> +&msmgpio {
>> + blsp1_uart2_default: blsp1_uart2_default {
>> + pinmux {
>> + function = "blsp_uart2";
>> + pins = "gpio4", "gpio5";
>> + };
>> + pinconf {
>> + pins = "gpio4", "gpio5";
>> + drive-strength = <16>;
>> + bias-disable;
>> + };
>> + };
>> +
>> + blsp1_uart2_sleep: blsp1_uart2_sleep {
>> + pinmux {
>> + function = "gpio";
>> + pins = "gpio4", "gpio5";
>> + };
>> + pinconf {
>> + pins = "gpio4", "gpio5";
>> + drive-strength = <2>;
>> + bias-pull-down;
>> + };
>> + };
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/msm8994-v2.0.dtsi b/arch/arm64/boot/dts/qcom/msm8994-v2.0.dtsi
>> new file mode 100644
>> index 0000000..8fc4c41f
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/msm8994-v2.0.dtsi
>> @@ -0,0 +1,31 @@
>> +/* Copyright (c) 2014-2016, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only 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.
>> + */
>> +
>> +/*
>> + * As a general rule, only version-specific property overrides should be placed
>> + * inside this file. Device definitions should be placed inside the msm8994.dtsi
>> + * file.
>> + */
>> +
>> +#include "msm8994.dtsi"
>> +
>> +/ {
>> + model = "Qualcomm Technologies, Inc. MSM 8994v2.0";
>> + compatible = "qcom,msm8994";
>> + qcom,msm-id = <207 0x20000>;
>
> Drop the msm-id
>
Dealt with over IRC.
>> +
>> +};
>> +
>> +/* Clock driver overrides */
>> +&clock_gcc {
>> + compatible = "qcom,gcc-8994v2";
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/msm8994.dtsi b/arch/arm64/boot/dts/qcom/msm8994.dtsi
>> new file mode 100644
>> index 0000000..c95cb73
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/msm8994.dtsi
>> @@ -0,0 +1,237 @@
>> +/* Copyright (c) 2013-2016, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only 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.
>> + */
>> +
>> +/memreserve/ 0x00000000 0x00001000;
>> +/memreserve/ 0xac1c0000 0x00001000;
>
> So the 0x00000000 is totally bogus. And the 0xac1c0000 needs to move to a
> reserved area lower in the dts.
>
I am going to leave this in until we get a chance to test the various
permutations etc... ideally we will be able to remove both completely.
This is explicitly mentioned in the leadin for v2.
>> +
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/clock/qcom,gcc-msm8994.h>
>> +
>> +/ {
>> + model = "Qualcomm Technologies, Inc. MSM 8994";
>> + compatible = "qcom,msm8994";
>> + qcom,msm-id = <207 0x0>;
>> + qcom,pmic-id = <0x10009 0x1000A 0x0 0x0>;
>
> kill the ids
>
done
>> + interrupt-parent = <&intc>;
>> +
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + chosen { };
>> +
>> + cpus {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + cpu-map {
>> + cluster0 {
>> + core0 {
>> + cpu = <&CPU0>;
>> + };
>> + };
>> + };
>> +
>> + CPU0: cpu at 0 {
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a53", "arm,armv8";
>> + reg = <0x0>;
>> + next-level-cache = <&L2_0>;
>> + // The currents(uA) correspond to the frequencies in the
>> + // frequency table.
>> + current = < 24140 //384000 kHZ
>> + 27200 //460800 kHZ
>> + 32300 //600000 kHZ
>> + 36940 //672000 kHz
>> + 41570 //768000 kHZ
>> + 49870 //864000 kHZ
>> + 57840 //960000 kHZ
>> + 79800 //1248000 kHZ
>> + 88810 //1344000 kHZ
>> + 102400 //1478400 kHZ
>> + 110900>; //1555200 kHZ
>> + L2_0: l2-cache {
>> + compatible = "cache";
>> + cache-level = <2>;
>> + };
>> + };
>> + };
>> +
>> + soc: soc { };
>> +
>> + memory {
>> + device_type = "memory";
>> + /* We expect the bootloader to fill in the reg */
>> + reg = <0 0 0 0>;
>> + };
>> +
>> + reserved-memory {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + ranges;
>
> Move that reserve down here and make it no-map.
>
See comment above on reserved memory.
<....snip>
-jeremy
^ permalink raw reply
* [PATCH] dmaengine: omap-dma: Enable burst and data pack for SG
From: Vinod Koul @ 2016-10-01 6:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160916105315.28320-1-peter.ujfalusi@ti.com>
On Fri, Sep 16, 2016 at 01:53:15PM +0300, Peter Ujfalusi wrote:
> From: Misael Lopez Cruz <misael.lopez@ti.com>
>
> Enable the burst and data pack modes for the scatter-gather
> in order to improve the throughput of the data transfers.
>
> The improvement has been verified with MMC HS200 mode in
> the DRA72 EVM using the iozone tool to compare the read
> throughput (in kB/s) with and without burst/pack for
> different reclens (in kB).
>
> With
> reclen Baseline sDMA burst/pack
> ------ -------- ---------------
> 64 46568 50820
> 128 57564 63413
> 256 65634 74937
> 512 72427 83483
> 1024 74563 84504
> 2048 76265 86079
> 4096 78045 87335
> 8192 78989 88154
> 16384 81265 91034
Applied, thanks
--
~Vinod
^ permalink raw reply
* [PATCH V4 04/10] dmaengine: qcom_hidma: configure DMA and MSI for OF
From: Vinod Koul @ 2016-10-01 6:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475115167-5898-5-git-send-email-okaya@codeaurora.org>
On Wed, Sep 28, 2016 at 10:12:41PM -0400, Sinan Kaya wrote:
> Configure the DMA bindings for the device tree based firmware.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/dma/qcom/hidma_mgmt.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/qcom/hidma_mgmt.c b/drivers/dma/qcom/hidma_mgmt.c
> index 82f36e4..e8f6b84 100644
> --- a/drivers/dma/qcom/hidma_mgmt.c
> +++ b/drivers/dma/qcom/hidma_mgmt.c
> @@ -375,8 +375,11 @@ static int __init hidma_mgmt_of_populate_channels(struct device_node *np)
> ret = PTR_ERR(new_pdev);
> goto out;
> }
> + of_node_get(child);
> + new_pdev->dev.of_node = child;
> of_dma_configure(&new_pdev->dev, child);
> -
> + of_msi_configure(&new_pdev->dev, child);
> + of_node_put(child);
should this be done unconditionally? Dont we needto call this only for
platforms with msi?
> kfree(res);
> res = NULL;
> }
> --
> 1.9.1
>
--
~Vinod
^ permalink raw reply
* [PATCH V4 05/10] dmaengine: qcom_hidma: make pending_tre_count atomic
From: Vinod Koul @ 2016-10-01 6:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475115167-5898-6-git-send-email-okaya@codeaurora.org>
On Wed, Sep 28, 2016 at 10:12:42PM -0400, Sinan Kaya wrote:
> Getting ready for the MSI interrupts. The pending_tre_count is used
> in the interrupt handler to make sure all outstanding requests are
> serviced.
>
> Making it atomic so that it can be updated from multiple contexts.
How is it multiple contexts? It's either existing context of MSI, not both!
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/dma/qcom/hidma.h | 2 +-
> drivers/dma/qcom/hidma_dbg.c | 3 ++-
> drivers/dma/qcom/hidma_ll.c | 13 ++++++-------
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/qcom/hidma.h b/drivers/dma/qcom/hidma.h
> index e52e207..3f2ddd4 100644
> --- a/drivers/dma/qcom/hidma.h
> +++ b/drivers/dma/qcom/hidma.h
> @@ -58,7 +58,7 @@ struct hidma_lldev {
> void __iomem *evca; /* Event Channel address */
> struct hidma_tre
> **pending_tre_list; /* Pointers to pending TREs */
> - s32 pending_tre_count; /* Number of TREs pending */
> + atomic_t pending_tre_count; /* Number of TREs pending */
>
> void *tre_ring; /* TRE ring */
> dma_addr_t tre_dma; /* TRE ring to be shared with HW */
> diff --git a/drivers/dma/qcom/hidma_dbg.c b/drivers/dma/qcom/hidma_dbg.c
> index fa827e5..87db285 100644
> --- a/drivers/dma/qcom/hidma_dbg.c
> +++ b/drivers/dma/qcom/hidma_dbg.c
> @@ -74,7 +74,8 @@ static void hidma_ll_devstats(struct seq_file *s, void *llhndl)
> seq_printf(s, "tre_ring_handle=%pap\n", &lldev->tre_dma);
> seq_printf(s, "tre_ring_size = 0x%x\n", lldev->tre_ring_size);
> seq_printf(s, "tre_processed_off = 0x%x\n", lldev->tre_processed_off);
> - seq_printf(s, "pending_tre_count=%d\n", lldev->pending_tre_count);
> + seq_printf(s, "pending_tre_count=%d\n",
> + atomic_read(&lldev->pending_tre_count));
> seq_printf(s, "evca=%p\n", lldev->evca);
> seq_printf(s, "evre_ring=%p\n", lldev->evre_ring);
> seq_printf(s, "evre_ring_handle=%pap\n", &lldev->evre_dma);
> diff --git a/drivers/dma/qcom/hidma_ll.c b/drivers/dma/qcom/hidma_ll.c
> index 3224f24..29fef4f 100644
> --- a/drivers/dma/qcom/hidma_ll.c
> +++ b/drivers/dma/qcom/hidma_ll.c
> @@ -218,10 +218,9 @@ static int hidma_post_completed(struct hidma_lldev *lldev, int tre_iterator,
> * Keep track of pending TREs that SW is expecting to receive
> * from HW. We got one now. Decrement our counter.
> */
> - lldev->pending_tre_count--;
> - if (lldev->pending_tre_count < 0) {
> + if (atomic_dec_return(&lldev->pending_tre_count) < 0) {
> dev_warn(lldev->dev, "tre count mismatch on completion");
> - lldev->pending_tre_count = 0;
> + atomic_set(&lldev->pending_tre_count, 0);
> }
>
> spin_unlock_irqrestore(&lldev->lock, flags);
> @@ -321,7 +320,7 @@ void hidma_cleanup_pending_tre(struct hidma_lldev *lldev, u8 err_info,
> u32 tre_read_off;
>
> tre_iterator = lldev->tre_processed_off;
> - while (lldev->pending_tre_count) {
> + while (atomic_read(&lldev->pending_tre_count)) {
> if (hidma_post_completed(lldev, tre_iterator, err_info,
> err_code))
> break;
> @@ -548,7 +547,7 @@ void hidma_ll_queue_request(struct hidma_lldev *lldev, u32 tre_ch)
> tre->err_code = 0;
> tre->err_info = 0;
> tre->queued = 1;
> - lldev->pending_tre_count++;
> + atomic_inc(&lldev->pending_tre_count);
> lldev->tre_write_offset = (lldev->tre_write_offset + HIDMA_TRE_SIZE)
> % lldev->tre_ring_size;
> spin_unlock_irqrestore(&lldev->lock, flags);
> @@ -654,7 +653,7 @@ int hidma_ll_setup(struct hidma_lldev *lldev)
> u32 val;
> u32 nr_tres = lldev->nr_tres;
>
> - lldev->pending_tre_count = 0;
> + atomic_set(&lldev->pending_tre_count, 0);
> lldev->tre_processed_off = 0;
> lldev->evre_processed_off = 0;
> lldev->tre_write_offset = 0;
> @@ -816,7 +815,7 @@ int hidma_ll_uninit(struct hidma_lldev *lldev)
> tasklet_kill(&lldev->task);
> memset(lldev->trepool, 0, required_bytes);
> lldev->trepool = NULL;
> - lldev->pending_tre_count = 0;
> + atomic_set(&lldev->pending_tre_count, 0);
> lldev->tre_write_offset = 0;
>
> rc = hidma_ll_reset(lldev);
> --
> 1.9.1
>
--
~Vinod
^ permalink raw reply
* [PATCH 6/6] ARM: da850: adjust memory settings for tilcdc
From: Sekhar Nori @ 2016-10-01 9:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <91db8934-bfd6-8fc4-74e3-4844ef3bd3e7@ti.com>
On Saturday 01 October 2016 12:49 AM, Peter Ujfalusi wrote:
> On 09/30/2016 06:06 PM, Bartosz Golaszewski wrote:
>> 2016-09-30 14:59 GMT+02:00 Peter Ujfalusi <peter.ujfalusi@ti.com>:
>>> On 09/29/16 19:31, Bartosz Golaszewski wrote:
>>>> Default memory settings of da850 do not meet the throughput/latency
>>>> requirements of tilcdc. This results in the image displayed being
>>>> incorrect and the following warning being displayed by the LCDC
>>>> drm driver:
>>>>
>>>> tilcdc da8xx_lcdc.0: tilcdc_crtc_irq(0x00000020): FIFO underfow
>>>>
>>>> Reconfigure the LCDC priority to the highest. This is a workaround
>>>> for the da850-lcdk board which has the LCD controller enabled in
>>>> the device tree, but a long-term, system-wide fix is needed for
>>>> all davinci boards.
>>>>
>>>> This patch has been modified for mainline linux. It comes from a
>>>> downstream TI release for da850[1].
>>>>
>>>> Original author: Vishwanathrao Badarkhe, Manish <manishv.b@ti.com>
>>>>
>>
>> [snip]
>>
>>>
>>> Is this safe to do for all da850 boards (to change PR_OLD_COUNT from 0xff to
>>> 0x20)? Most probably it is, but this setting has nothing to do with LCDC.
>>>
>>> The whole priority configuration has nothing to do with the LCDC, it is a
>>> system level priority.
>>>
>>> Now you have lowered the eDMA3_0-TPTC0/1 priority. Audio is serviced by
>>> eDMA3_0-TPTC1. So are we going to see issues in audio if LCDC is taking the
>>> highest priority?
>>>
>>
>> Just ran a quick test with speaker-test -c2 -twav. Besides the fact
>> that the left and right channels are inverted (I'm looking into that),
>> I didn't notice any problems. Even at 1024x768 resolution, playing
>> audio at the same time seems to work fine.
>
> That's good to hear, but I think the priorities should be set:
> LCDC and EDMA30TC1 to highest priority
> EDMA30TC0 to priority 2
>
> The 0TC0 is used by MMC and if you want to play a video you might need the
> servicing TC to be higher priority then other masters.
>
> If audio playback would trigger sync losts in lcdc then we might need to move
> 0TC1 to priority 1.
>
> I agree that LCDC priority needs to be higher, but I do wonder why the default
> (5) is not working and if it is not working why it is 5...
>
> My guess is that the change in the PBBPR register is the one actually helping
> here.
Good point, Peter. If you are booting off NFS and not playing any audio,
then there is pretty much no EDMA generated traffic on the interconnect.
I would guess too that its the PBBPR setting that is making a
difference. The EDMA vs LCDC priority adjustment might be needed in
particular situations too, but specific experiments should be done to
narrow down on that being the cause.
In any case, to configure the PBBR, you will have to introduce a driver
for it in drivers/memory. Then you can set it up per board using a DT
parameter.
Thanks,
Sekhar
^ permalink raw reply
* [PATCH] mtd: mtk: avoid warning in mtk_ecc_encode
From: Boris Brezillon @ 2016-10-01 9:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201609301925.17577.arnd@arndb.de>
On Fri, 30 Sep 2016 19:25:17 +0200
Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 30 September 2016, Boris Brezillon wrote:
> > > + /* copy into possibly unaligned OOB region with actual length */
> > > + memcpy(data + bytes, eccdata, len);
> >
> > Is it better than
> >
> > for (i = 0; i < len; i += 4) {
> > u32 val = __raw_readl(ecc->regs + ECC_ENCPAR(i / 4));
> >
> > memcpy(data + bytes + i, &val, min(len, 4));
> > }
> >
> > I'm probably missing something, but what's the point of creating a
> > temporary buffer of 112 bytes on the stack since you'll have to copy
> > this data to the oob buffer at some point?
>
>
> I tried something like that first, but wasn't too happy with it for
> a number of small reasons:
>
> - __raw_readl in a driver is not usually the right API, __memcpy32_from_io
> uses it internally, but it's better for a driver not to rely on that,
> in case we need some barriers (which we may in factt need for other drivers).
I agree, even though calling something prefixed with __ (in this case,
__ioread32_copy()) sounds like a bad thing too :).
>
> - the min(len,4) expression is incorrect, fixing that makes it more complicated
> again
Sorry, it's min(len - i, 4), which is not that complicated :P.
>
> - I didn't like to call memcpy() multiple times, as that might get turned
> into an external function call (the compiler is free to optimize small
> memcpy calls or not).
Okay.
>
> I agree that he 112 byte buffer isn't ideal either, it just seemed to
> be the lesser annoyance.
How about we keep your approach, but put the buffer in the mtk_ecc
struct?
^ permalink raw reply
* [PATCH v2] drm: tilcdc: add a da850-specific compatible string
From: Sekhar Nori @ 2016-10-01 9:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475243564-15765-1-git-send-email-bgolaszewski@baylibre.com>
On Friday 30 September 2016 07:22 PM, Bartosz Golaszewski wrote:
> Due to some potential tweaks for the da850 LCDC (for example: the
> required memory bandwith settings) we need a separate compatible
> for the IP present on the da850 boards.
>
> Suggested-by: Sekhar Nori <nsekhar@ti.com>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> v1 -> v2:
> - added the new compatible to the bindings documentation
>
> Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt | 4 ++--
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 1 +
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt b/Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt
> index a83abd7..33b6e8a 100644
> --- a/Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt
> +++ b/Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt
> @@ -1,7 +1,7 @@
> Device-Tree bindings for tilcdc DRM driver
>
> Required properties:
> - - compatible: value should be "ti,am33xx-tilcdc".
> + - compatible: value should be "ti,am33xx-tilcdc" or "ti,da850-tilcdc".
This documentation does not help much :( It should be on the lines of:
compatible: value should be "ti,am33xx-tilcdc" for AM335x based boards
value should be "ti,da850-tilcdc" for DA850/AM18x/OMAP-L138
based boards
There are many existing examples of the way compatible strings are
documented. You can take a look at them too. Also, since you are
introducing a new device-tree compatible, please keep the devicetree
list and maintainers in CC too. I don't think it shows up in
get_maintainer.pl, so you will have to remember to do it.
Thanks,
Sekhar
^ permalink raw reply
* [PATCH 4/6] ARM: dts: da850-lcdk: add support for 1024x768 resolution
From: Sekhar Nori @ 2016-10-01 9:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMpxmJX25Yx0Rget4bOUf+t2qn01bpbNGBwwVFfomK1h_2Oydg@mail.gmail.com>
On Friday 30 September 2016 07:18 PM, Bartosz Golaszewski wrote:
> 2016-09-30 14:49 GMT+02:00 Karl Beldan <karl.beldan@gmail.com>:
>> On Fri, Sep 30, 2016 at 11:37:57AM +0200, Bartosz Golaszewski wrote:
>>> 2016-09-29 20:58 GMT+02:00 Karl Beldan <karl.beldan@gmail.com>:
>>>> Hi,
>>>>
>>>> On Thu, Sep 29, 2016 at 06:31:53PM +0200, Bartosz Golaszewski wrote:
>>>>> Add svga timings for 1024x768 resolution to the da850-lcdk
>>>>> device tree.
>>>>>
>>>>
>>>
>>> [snip]
>>>
>>>>
>>>> Why do you also call 1024x768 svga ?
>>>>
>>>
>>> Thanks, should have been xga. will fix in v2.
>>>
>>>> I don't think the LCDK can cope with this resolution at this frequency
>>>> (in terms of mem bandwidth), at least that's what I observed back in
>>>> August. If confirmed I think it is worth mentioning in the log at least,
>>>> but then I doubt adding this config would be useful.
>>>>
>>>
>>> Thanks for the heads up. How would that manifest itself? This seems to
>>> work fine for me - I'm not getting any warnings on a simple system -
>>> maybe if I added some additional memory load it would complain.
>>>
>>
>> A mere dmesg > /dev/tty0 (or repeatedly cat-ting a file to /dev/tty0)
>> should suffice to make the issue visible and trigger FIFO underflows.
>>
>
> Thanks, this is correct unfortunately.
The OMAP-L138 datasheet documents 37.5 MHz as the maximum pixel clock
possible on the device. Looks like you need 72MHz pixel clock for
1024x768 which is definitely out of spec.
Thanks,
Sekhar
^ permalink raw reply
* [PATCH v2 0/6] Support PWM polarity control
From: Bhuvanchandra DV @ 2016-10-01 10:12 UTC (permalink / raw)
To: linux-arm-kernel
Changes since v2:
- Picked the stalled patchset[1] from Lothar Wassmann which adds the basic
support for polarity control on imx-pwm driver and adds backward compatibility
support for devices which does not have polarity control feature.
Changes since Lothars v6:
- Squash Lukasz patch[2].
[1] http://thread.gmane.org/gmane.linux.pwm/1621
[2] https://www.spinics.net/lists/arm-kernel/msg530818.html
Bhuvanchandra DV (3):
arm: dts: imx7: Update #pwm-cells for PWM polarity control
arm: dts: imx7-colibri: Use pwm polarity control
arm: dts: imx7-colibri: Use enable-gpios for BL_ON
Lothar Wassmann (3):
pwm: print error messages with pr_err() instead of pr_debug()
pwm: core: make the PWM_POLARITY flag in DTB optional
pwm: imx: support output polarity inversion
Documentation/devicetree/bindings/pwm/imx-pwm.txt | 6 +--
arch/arm/boot/dts/imx7-colibri.dtsi | 12 +++++-
arch/arm/boot/dts/imx7s.dtsi | 8 ++--
drivers/pwm/core.c | 31 ++++++++------
drivers/pwm/pwm-imx.c | 51 +++++++++++++++++++++--
5 files changed, 83 insertions(+), 25 deletions(-)
--
2.9.2
^ permalink raw reply
* [PATCH v2 1/6] pwm: print error messages with pr_err() instead of pr_debug()
From: Bhuvanchandra DV @ 2016-10-01 10:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161001101235.24598-1-bhuvanchandra.dv@toradex.com>
From: Lothar Wassmann <LW@KARO-electronics.de>
Make the messages that are printed in case of fatal errors actually visible to
the user without having to recompile the driver with debugging enabled.
Signed-off-by: Lothar Wa?mann <LW@KARO-electronics.de>
Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
---
drivers/pwm/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 0dbd29e..195373e 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -661,13 +661,13 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id)
err = of_parse_phandle_with_args(np, "pwms", "#pwm-cells", index,
&args);
if (err) {
- pr_debug("%s(): can't parse \"pwms\" property\n", __func__);
+ pr_err("%s(): can't parse \"pwms\" property\n", __func__);
return ERR_PTR(err);
}
pc = of_node_to_pwmchip(args.np);
if (IS_ERR(pc)) {
- pr_debug("%s(): PWM chip not found\n", __func__);
+ pr_err("%s(): PWM chip not found\n", __func__);
pwm = ERR_CAST(pc);
goto put;
}
--
2.9.2
^ permalink raw reply related
* [PATCH v2 2/6] pwm: core: make the PWM_POLARITY flag in DTB optional
From: Bhuvanchandra DV @ 2016-10-01 10:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161001101235.24598-1-bhuvanchandra.dv@toradex.com>
From: Lothar Wassmann <LW@KARO-electronics.de>
Change the pwm chip driver registration, so that a chip driver that supports
polarity inversion can still be used with DTBs that don't provide the
'PWM_POLARITY' flag.
This is done to provide polarity inversion support for the pwm-imx driver
without having to modify all existing DTS files.
Signed-off-by: Lothar Wassmann <LW@KARO-electronics.de>
Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
Suggested-by: Thierry Reding <thierry.reding@gmail.com>
---
drivers/pwm/core.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 195373e..aae8db3 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -137,9 +137,14 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
{
struct pwm_device *pwm;
+ /* check, whether the driver supports a third cell for flags */
if (pc->of_pwm_n_cells < 3)
return ERR_PTR(-EINVAL);
+ /* flags in the third cell are optional */
+ if (args->args_count < 2)
+ return ERR_PTR(-EINVAL);
+
if (args->args[0] >= pc->npwm)
return ERR_PTR(-EINVAL);
@@ -149,10 +154,12 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
pwm->args.period = args->args[1];
- if (args->args[2] & PWM_POLARITY_INVERTED)
- pwm->args.polarity = PWM_POLARITY_INVERSED;
- else
- pwm->args.polarity = PWM_POLARITY_NORMAL;
+ if (args->args_count > 2) {
+ if (args->args[2] & PWM_POLARITY_INVERTED)
+ pwm_set_polarity(pwm, PWM_POLARITY_INVERSED);
+ else
+ pwm_set_polarity(pwm, PWM_POLARITY_NORMAL);
+ }
return pwm;
}
@@ -163,9 +170,14 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
{
struct pwm_device *pwm;
+ /* sanity check driver support */
if (pc->of_pwm_n_cells < 2)
return ERR_PTR(-EINVAL);
+ /* all cells are required */
+ if (args->args_count != pc->of_pwm_n_cells)
+ return ERR_PTR(-EINVAL);
+
if (args->args[0] >= pc->npwm)
return ERR_PTR(-EINVAL);
@@ -672,13 +684,6 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id)
goto put;
}
- if (args.args_count != pc->of_pwm_n_cells) {
- pr_debug("%s: wrong #pwm-cells for %s\n", np->full_name,
- args.np->full_name);
- pwm = ERR_PTR(-EINVAL);
- goto put;
- }
-
pwm = pc->of_xlate(pc, &args);
if (IS_ERR(pwm))
goto put;
--
2.9.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox