* [PATCH 2/4 v7] drm/bridge: Provide a way to embed timing info in bridges
From: Laurent Pinchart @ 2018-01-12 22:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180112074854.9560-2-linus.walleij@linaro.org>
Hi Linus,
Thank you for the patch.
On Friday, 12 January 2018 09:48:52 EET Linus Walleij wrote:
> After some discussion and failed patch sets trying to convey
> the right timing information between the display engine and
> a bridge using the connector, I try instead to use an optional
> timing information container in the bridge itself, so that
> display engines can retrieve it from any bridge and use it to
> determine how to drive outputs.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> ChangeLog v6->v7:
> - Fix the comment style to use the new inline type of kerneldoc
> for struct members.
> - Need an explicit ACK/review by someone on this patch to continue
> with the series...
> ChangeLog v5->v6:
> - Sort forward struct declarations alphabetically
> - Switch to using DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE to indicate
> positive or negatice clock samling edge
> ChangeLog ->v5:
> - New patch
> ---
> include/drm/drm_bridge.h | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 682d01ba920c..d3c2eea0bb63 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -29,6 +29,7 @@
> #include <drm/drm_modes.h>
>
> struct drm_bridge;
> +struct drm_bridge_timings;
> struct drm_panel;
>
> /**
> @@ -222,6 +223,35 @@ struct drm_bridge_funcs {
> void (*enable)(struct drm_bridge *bridge);
> };
>
> +/**
> + * struct drm_bridge_timings - timing information for the bridge
> + */
> +struct drm_bridge_timings {
> + /**
> + * @sampling_edge:
> + *
> + * Tells whether the bridge samples the digital input signal
> + * from the display engine on the positive or negative edge of the clock,
> + * this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE bitwise
> + * flags from the DRM connector (bit 2 and 3 valid).
> + */
> + u32 sampling_edge;
> + /**
> + * @setup_time_ps:
> + *
> + * Defines the time in picoseconds the input data lines must be
> + * stable before the clock edge.
> + */
> + u32 setup_time_ps;
> + /**
> + * @hold_time_ps:
> + *
> + * Defines the time in picoseconds taken for the bridge to sample the
> + * input signal after the clock edge.
> + */
> + u32 hold_time_ps;
> +};
> +
> /**
> * struct drm_bridge - central DRM bridge control structure
> * @dev: DRM device this bridge belongs to
> @@ -229,6 +259,8 @@ struct drm_bridge_funcs {
> * @next: the next bridge in the encoder chain
> * @of_node: device node pointer to the bridge
> * @list: to keep track of all added bridges
> + * @timings: the timing specification for the bridge, if any (may
> + * be NULL)
> * @funcs: control functions
> * @driver_private: pointer to the bridge driver's internal context
> */
> @@ -240,6 +272,7 @@ struct drm_bridge {
> struct device_node *of_node;
> #endif
> struct list_head list;
> + const struct drm_bridge_timings *timings;
>
> const struct drm_bridge_funcs *funcs;
> void *driver_private;
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt
From: Keerthy @ 2018-01-12 22:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1da42501-5a34-0e4d-68bd-1712b63174f8@ti.com>
On 1/13/2018 2:47 AM, Grygorii Strashko wrote:
>
>
> On 01/12/2018 03:08 PM, Tony Lindgren wrote:
>> * Grygorii Strashko <grygorii.strashko@ti.com> [180112 20:36]:
>>> Hi
>>>
>>> On 01/10/2018 10:54 PM, Qi Hou wrote:
>>>> When more than one GP timers are used as kernel system timers and the
>>>> corresponding nodes in device-tree are marked with the same "disabled"
>>>> property, then the "attr" field of the property will be initialized
>>>> more than once as the property being added to sys file system via
>>>> __of_add_property_sysfs().
>>>>
>>>> In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
>>>> directly to the return value of safe_name(), without taking care of
>>>> whether it's already a valid pointer to a memory block. If it is, its
>>>> old value will always be overwritten by the new one and the memory
>>>> block
>>>> allocated before will a "ghost", then a kmemleak happened.
>>>>
>>>> That the same "disabled" property being added to different nodes of
>>>> device
>>>> tree would cause that kind of kmemleak overhead, at leat once.
>>>>
>>>> To fix it, allocate the property dynamically, and delete static one.
>>>
>>> Does it in sync with Keerthy's work [1]
>>
>> First fixes, then new stuff! Keerthy's work will have to
>> wait for v4.17, we want that series sitting in Linux next
>> for several weeks.
>>
> Np. just want to be sure every party is aware about each other's work
Thanks Grygorii. I am not touching the mach-omap2 timer.c file as part
of my migration series.
I applied the above patch and it seems my series applies cleanly after
this patch. Compiled fine.
>
^ permalink raw reply
* PM regression in next
From: Mark Brown @ 2018-01-12 22:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180112215010.GG4821@atomide.com>
On Fri, Jan 12, 2018 at 01:50:10PM -0800, Tony Lindgren wrote:
> > I rather suspect someone would've noticed by now TBH - I suspect this is
> > more likely to be an issue with the rather baroque code that the TWL
> > drivers have possibly coupled with the whole multiple I2C devices issue.
> Hmm these calls are in sound/soc/soc-core.c though? But yeah, sure
> it could be some legacy code issue somewhere..
Most devices have one regmap per device which can be retrieved with
dev_get_regmap(), it's the attempt to use that which I suspect is
broken. Like I said snd_soc_codec_init_regmap() ought to fix things if
that's the issue.
> sound/soc/codecs/cx20442.c
> sound/soc/codecs/tlv320dac33.c
> sound/soc/codecs/twl4030.c
> sound/soc/codecs/twl6040.c
> sound/soc/codecs/uda1380.c
> sound/soc/fsl/fsl_ssi.c
> Can you confirm that some of these are still working?
Most of the non-TWL ones have no active users I'm aware of but fsl_ssi.c
is under active development at the minute, one series was getting some
successful testing earlier today so if it's broken I'd hope someone
would have noticed although since it doesn't use DAPM or anything
snd_soc_read()/write() will never get used. cx20442 was only in a
single archaic consumer product, uda1380 has been legacy since before I
was ever involved in ASoC.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180112/f25ed362/attachment.sig>
^ permalink raw reply
* [PATCH 1/3] arm: spear600: Add missing interrupt-parent of rtc
From: Arnd Bergmann @ 2018-01-12 22:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOesGMiHeitS_j9r0DsPf3k1jVJT9_z-bHyrhbB7fQ__LjjaqA@mail.gmail.com>
On Fri, Jan 12, 2018 at 10:57 PM, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Jan 12, 2018 at 1:53 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Fri, Jan 12, 2018 at 10:50 PM, Olof Johansson <olof@lixom.net> wrote:
>>> On Fri, Jan 12, 2018 at 12:45 PM, Rob Herring <robh+dt@kernel.org> wrote:
>>>> On Fri, Jan 12, 2018 at 12:23 PM, Olof Johansson <olof@lixom.net> wrote:
>>>> I have more dtc checks in the works (nothing for 4.16 :) ). I'd like
>>>> the process to work better. I'm not going to fix all the warnings. I
>>>> don't think Arnd should either. Turning them off by default hasn't
>>>> worked great either. For some, I'm not sure we can ever get to warning
>>>> free, but I'd like new stuff to have warnings enabled and no one
>>>> builds with W=1. We could put together tooling to just show new
>>>> warnings, but someone has to run it and enforce it. I could stick dtc
>>>> updates into linux-next for multiple cycles before sending to Linus.
>>>
>>> I'll split up and report DT warnings separate from compiler, seems
>>> like a reasonable approach.
>>
>> Maybe also report any other output from the build process as another
>> category. When things build fine, we should see no output at all.
>
> Yep, true.
>
> Right now there's mostly DTC warnings and missing MODULE_LICENSE().
I've sent patches for all the remaining MODULE_LICENSE warnings earlier
this week, and we should now have patches for almost all the dtc
warnings in flight. This is what I still see on my randconfig builder
with linux-next + arm-soc/for-next + patches I sent:
arch/arm/boot/dts/spear1310-evb.dtb: Warning (gpios_property):
Missing property '#gpio-cells' in node /interrupt-controller at ec801000
or bad phandle (referred from /ahb/apb/spi at e0100000:cs-gpios[6])
arch/arm/boot/dts/spear1310-evb.dtb: Warning (gpios_property):
Property 'cs-gpios', cell 6 is not a phandle reference in
/ahb/apb/spi at e0100000
arch/arm/boot/dts/spear1340-evb.dtb: Warning (dmas_property):
Missing property '#dma-cells' in node /interrupt-controller at ec801000
or bad phandle (referred from /ahb/apb/serial at b4100000:dmas[4])
arch/arm/boot/dts/spear1340-evb.dtb: Warning (dmas_property):
Property 'dmas', cell 4 is not a phandle reference in
/ahb/apb/serial at b4100000
arch/arm/boot/dts/spear600-evb.dtb: Warning
(interrupts_property): Missing interrupt-parent for
/ahb/apb/rtc at fc900000
arch/arm/boot/dts/ste-nomadik-nhk15.dtb: Warning
(interrupts_property): Missing interrupt-parent for
/amba/clcd at 10120000
arch/arm/boot/dts/ste-nomadik-s8815.dtb: Warning
(interrupts_property): Missing interrupt-parent for
/amba/clcd at 10120000
Not sure why the spear warnings are still there, maybe Viresh missed those
or you missed one of his patches?
I hope Linus Walleij can find out what the right interrupt-parent
should be on the
nomadik machine, that is otherwise the last remaining warning. Apparently the
clcd driver doesn't actually need its interrupt, so we could decide to
just comment
out that line if we don't know which controller it's connected to.
Arnd
^ permalink raw reply
* [PATCH 1/3] arm: spear600: Add missing interrupt-parent of rtc
From: Olof Johansson @ 2018-01-12 21:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAK8P3a3KTz0sOSbO9+sAY1Sv95hZF4Yg1qMzOaXPAZ_1pOgGpg@mail.gmail.com>
On Fri, Jan 12, 2018 at 1:53 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Jan 12, 2018 at 10:50 PM, Olof Johansson <olof@lixom.net> wrote:
>> On Fri, Jan 12, 2018 at 12:45 PM, Rob Herring <robh+dt@kernel.org> wrote:
>>> On Fri, Jan 12, 2018 at 12:23 PM, Olof Johansson <olof@lixom.net> wrote:
>>> I have more dtc checks in the works (nothing for 4.16 :) ). I'd like
>>> the process to work better. I'm not going to fix all the warnings. I
>>> don't think Arnd should either. Turning them off by default hasn't
>>> worked great either. For some, I'm not sure we can ever get to warning
>>> free, but I'd like new stuff to have warnings enabled and no one
>>> builds with W=1. We could put together tooling to just show new
>>> warnings, but someone has to run it and enforce it. I could stick dtc
>>> updates into linux-next for multiple cycles before sending to Linus.
>>
>> I'll split up and report DT warnings separate from compiler, seems
>> like a reasonable approach.
>
> Maybe also report any other output from the build process as another
> category. When things build fine, we should see no output at all.
Yep, true.
Right now there's mostly DTC warnings and missing MODULE_LICENSE().
-Olof
^ permalink raw reply
* [PATCH 1/3] arm: spear600: Add missing interrupt-parent of rtc
From: Arnd Bergmann @ 2018-01-12 21:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOesGMiyfpRyoe4dxQeNMF5invysVBEy+J5OwUhuop=JyAvZoQ@mail.gmail.com>
On Fri, Jan 12, 2018 at 10:50 PM, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Jan 12, 2018 at 12:45 PM, Rob Herring <robh+dt@kernel.org> wrote:
>> On Fri, Jan 12, 2018 at 12:23 PM, Olof Johansson <olof@lixom.net> wrote:
>> I have more dtc checks in the works (nothing for 4.16 :) ). I'd like
>> the process to work better. I'm not going to fix all the warnings. I
>> don't think Arnd should either. Turning them off by default hasn't
>> worked great either. For some, I'm not sure we can ever get to warning
>> free, but I'd like new stuff to have warnings enabled and no one
>> builds with W=1. We could put together tooling to just show new
>> warnings, but someone has to run it and enforce it. I could stick dtc
>> updates into linux-next for multiple cycles before sending to Linus.
>
> I'll split up and report DT warnings separate from compiler, seems
> like a reasonable approach.
Maybe also report any other output from the build process as another
category. When things build fine, we should see no output at all.
Arnd
^ permalink raw reply
* [PATCH 1/3] arm: spear600: Add missing interrupt-parent of rtc
From: Olof Johansson @ 2018-01-12 21:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL_JsqJVWLazDRB1me2OubqqMhdsqsR+cLSHR+K0OY3dZHcE-w@mail.gmail.com>
On Fri, Jan 12, 2018 at 12:45 PM, Rob Herring <robh+dt@kernel.org> wrote:
> On Fri, Jan 12, 2018 at 12:23 PM, Olof Johansson <olof@lixom.net> wrote:
>> On Fri, Jan 12, 2018 at 12:56 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Fri, Jan 12, 2018 at 4:23 AM, Olof Johansson <olof@lixom.net> wrote:
>>>> On Thu, Jan 11, 2018 at 7:22 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>>>> On 11-01-18, 18:07, Olof Johansson wrote:
>>>>>> On Thu, Jan 11, 2018 at 11:28:51AM +0530, Viresh Kumar wrote:
>>>>>> > The interrupt-parent of rtc was missing, add it.
>>>>>> >
>>>>>> > Fixes: 8113ba917dfa ("ARM: SPEAr: DT: Update device nodes")
>>>>>> > Cc: stable at vger.kernel.org # v3.8+
>>>>>> > Reported-by: Arnd Bergmann <arnd@arndb.de>
>>>>>> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>>>>>>
>>>>>> Applied to next/dt. Is stable really needed on this? It's been broken since
>>>>>> pretty much forever, and nobody has complained... :)
>>>>>
>>>>> Not sure. Just thought it may be useful for someone somewhere :)
>>>>
>>>> Ok. Left the tags there, but didn't merge into fixes since we're late
>>>> in -rc and this didn't seem critical at this time.
>>>
>>> My plan was to have these in the fixes branch in the hope of making
>>> it to a clean build for 4.15 after all, they fix warnings that got introduced
>>> by the updated dtc checks in 4.15-rc1.
>>>
>>> We are getting fairly close, but it seems we still miss a few, so we
>>> might as well give up at this point. The remaining fixes should be easy
>>> to backport into v4.15.y if we decide to do it, of further back even.
>>> For v4.14 and before, the in-kernel copy of dtc won't warn, but mainline
>>> dtc will.
>>>
>>> Greg, let me know your thoughts on this for the upcoming 4.15.y
>>> release. We had hundreds of dtc warnings in 4.15-rc1, many of them
>>> about important bugs, now we're down to a couple of warnings
>>> for platforms we don't care about much, and I expect the last of
>>> these fixes to land in 4.16-rc1 or maybe -rc2. Shall we backport
>>> them all to get a clean 4.15.y release?
>>
>> I think it makes more sense to disable the warnings than to backport a
>> bunch of warning fixes this late. The code is working, has worked for
>> a long time it's just that Rob enabled the warnings by default. We can
>> keep them enabled for 4.16.
>
> In some cases "working" was what's in the DT is unused by the kernel
> because the DT is broken. That's why this round was not off by
> default.
>
> It looks to me to be somewhere less than 5 fixes remaining (BTW, why
> is there no arm32 allmodconfig build on kernelci.org? That or
> allyesconfig are the only ways to build all dtbs). It would also be an
> exception to the the stable process because that patch would not be in
> Linus' tree.
I build them but my script that analyses for warnings only looks for
the gcc "warning:" output. Need to do grep -i to catch them.
FWIW, logs are here:
http://arm-soc.lixom.net/buildlogs/mainline/v4.15-rc7-200-gc92a9a4/buildall.arm.allmodconfig.log.passed
>>> Note: there was at least one dtc warning fix that caused a serious
>>> regression in code that relied on a device probe to fail because of
>>> an invalid node (a fix is still in the works for 4.15), though generally
>>> the fixes are really harmless and can only make things better.
>>
>> Exactly why picking up warning fixes this late is probably not a great idea.
>
> Applying them now for 4.15 would be late, but tagging them as fixes in
> 4.16 would not be late (other than the normal problem that things get
> applied to stable before sufficient testing in master).
>
>
> I have more dtc checks in the works (nothing for 4.16 :) ). I'd like
> the process to work better. I'm not going to fix all the warnings. I
> don't think Arnd should either. Turning them off by default hasn't
> worked great either. For some, I'm not sure we can ever get to warning
> free, but I'd like new stuff to have warnings enabled and no one
> builds with W=1. We could put together tooling to just show new
> warnings, but someone has to run it and enforce it. I could stick dtc
> updates into linux-next for multiple cycles before sending to Linus.
I'll split up and report DT warnings separate from compiler, seems
like a reasonable approach.
-Olof
^ permalink raw reply
* PM regression in next
From: Tony Lindgren @ 2018-01-12 21:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180112211535.GH21458@sirena.org.uk>
* Mark Brown <broonie@kernel.org> [180112 21:15]:
> On Fri, Jan 12, 2018 at 01:07:06PM -0800, Tony Lindgren wrote:
>
> > (twl4030_dummy_read [snd_soc_twl4030]) from [<bf1cc3b4>]
> > (snd_soc_codec_drv_read+0x1c/0x28 [snd_soc_core])
> > (snd_soc_codec_drv_read [snd_soc_core]) from [<bf1da1cc>]
> > (snd_soc_dapm_new_widgets+0x29c/0x578 [snd_soc_core])
> > (snd_soc_dapm_new_widgets [snd_soc_core]) from [<bf1d2f30>]
> > (snd_soc_register_card+0xad0/0xe30 [snd_soc_core])
> > (snd_soc_register_card [snd_soc_core]) from [<bf1e1850>]
> > (devm_snd_soc_register_card+0x30/0x70 [snd_soc_core])
> > (devm_snd_soc_register_card [snd_soc_core]) from [<bf234364>]
> > (omap_twl4030_probe+0x100/0x1d0 [snd_soc_omap_twl4030])
> > (omap_twl4030_probe [snd_soc_omap_twl4030]) from [<c0606660>]
> > (platform_drv_probe+0x50/0xb0)
>
> So does the read not happen otherwise? Or is it failing somehow and the
> error getting ignored?
All the snd_soc_dapm_new_widgets() related reads go missing
without the .read being there. Probably the same story for
writes.
> > So probably there are other asoc drivers broken too with these
> > kind of patches until snd_soc_codec_drv_write() and
> > snd_soc_codec_drv_read() are fixed?
>
> I rather suspect someone would've noticed by now TBH - I suspect this is
> more likely to be an issue with the rather baroque code that the TWL
> drivers have possibly coupled with the whole multiple I2C devices issue.
Hmm these calls are in sound/soc/soc-core.c though? But yeah, sure
it could be some legacy code issue somewhere..
Based on a quick grep these have .read and .write removed in next
so they may all be broken now:
sound/soc/codecs/cx20442.c
sound/soc/codecs/tlv320dac33.c
sound/soc/codecs/twl4030.c
sound/soc/codecs/twl6040.c
sound/soc/codecs/uda1380.c
sound/soc/fsl/fsl_ssi.c
Can you confirm that some of these are still working?
Regards,
Tony
^ permalink raw reply
* PM regression in next
From: Mark Brown @ 2018-01-12 21:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180112210706.GE4821@atomide.com>
On Fri, Jan 12, 2018 at 01:07:06PM -0800, Tony Lindgren wrote:
> Tturns out just adding back .read = twl4030_read fixes it..
> I added a dummy function for read and am now seeing a bunch
> of reads that now don't happen:
What's supposed to happen here is that you end up with a
component->regmap set and snd_soc_component_read() uses that to do the
read. That normally gets retrieved via dev_get_regmap() but it looks
like this is broken here by the rather novel chip design confusing the
assumptions that code makes. I *think* that setting the regmap to
twl_get_regmap(TWL4030_MODULE_AUDIO_VOICE) using
snd_soc_codec_init_regmap() in twl4030_soc_probe ought to do the trick
(assuming my guesses about what's going on are right) but can't test.
I'd check twl6040 too, I bet it's got the same issue.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180112/b340cdc0/attachment.sig>
^ permalink raw reply
* [PATCH net-next 1/2 v11] net: ethernet: Add DT bindings for the Gemini ethernet
From: Linus Walleij @ 2018-01-12 21:34 UTC (permalink / raw)
To: linux-arm-kernel
This adds the device tree bindings for the Gemini ethernet
controller. It is pretty straight-forward, using standard
bindings and modelling the two child ports as child devices
under the parent ethernet controller device.
Cc: devicetree at vger.kernel.org
Cc: Tobias Waldvogel <tobias.waldvogel@gmail.com>
Cc: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v10->v11:
- Resend with the driver.
ChangeLog v9->v10:
- Resend with the driver.
ChangeLog v8->v9:
- Collect Rob's ACK.
ChangeLog v7->v8:
- Use ethernet-port at 0 and ethernet-port at 1 with unit names
and following OF graph requirements.
---
.../bindings/net/cortina,gemini-ethernet.txt | 92 ++++++++++++++++++++++
1 file changed, 92 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/cortina,gemini-ethernet.txt
diff --git a/Documentation/devicetree/bindings/net/cortina,gemini-ethernet.txt b/Documentation/devicetree/bindings/net/cortina,gemini-ethernet.txt
new file mode 100644
index 000000000000..6c559981d110
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/cortina,gemini-ethernet.txt
@@ -0,0 +1,92 @@
+Cortina Systems Gemini Ethernet Controller
+==========================================
+
+This ethernet controller is found in the Gemini SoC family:
+StorLink SL3512 and SL3516, also known as Cortina Systems
+CS3512 and CS3516.
+
+Required properties:
+- compatible: must be "cortina,gemini-ethernet"
+- reg: must contain the global registers and the V-bit and A-bit
+ memory areas, in total three register sets.
+- syscon: a phandle to the system controller
+- #address-cells: must be specified, must be <1>
+- #size-cells: must be specified, must be <1>
+- ranges: should be state like this giving a 1:1 address translation
+ for the subnodes
+
+The subnodes represents the two ethernet ports in this device.
+They are not independent of each other since they share resources
+in the parent node, and are thus children.
+
+Required subnodes:
+- port0: contains the resources for ethernet port 0
+- port1: contains the resources for ethernet port 1
+
+Required subnode properties:
+- compatible: must be "cortina,gemini-ethernet-port"
+- reg: must contain two register areas: the DMA/TOE memory and
+ the GMAC memory area of the port
+- interrupts: should contain the interrupt line of the port.
+ this is nominally a level interrupt active high.
+- resets: this must provide an SoC-integrated reset line for
+ the port.
+- clocks: this should contain a handle to the PCLK clock for
+ clocking the silicon in this port
+- clock-names: must be "PCLK"
+
+Optional subnode properties:
+- phy-mode: see ethernet.txt
+- phy-handle: see ethernet.txt
+
+Example:
+
+mdio-bus {
+ (...)
+ phy0: ethernet-phy at 1 {
+ reg = <1>;
+ device_type = "ethernet-phy";
+ };
+ phy1: ethernet-phy at 3 {
+ reg = <3>;
+ device_type = "ethernet-phy";
+ };
+};
+
+
+ethernet at 60000000 {
+ compatible = "cortina,gemini-ethernet";
+ reg = <0x60000000 0x4000>, /* Global registers, queue */
+ <0x60004000 0x2000>, /* V-bit */
+ <0x60006000 0x2000>; /* A-bit */
+ syscon = <&syscon>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ gmac0: ethernet-port at 0 {
+ compatible = "cortina,gemini-ethernet-port";
+ reg = <0x60008000 0x2000>, /* Port 0 DMA/TOE */
+ <0x6000a000 0x2000>; /* Port 0 GMAC */
+ interrupt-parent = <&intcon>;
+ interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
+ resets = <&syscon GEMINI_RESET_GMAC0>;
+ clocks = <&syscon GEMINI_CLK_GATE_GMAC0>;
+ clock-names = "PCLK";
+ phy-mode = "rgmii";
+ phy-handle = <&phy0>;
+ };
+
+ gmac1: ethernet-port at 1 {
+ compatible = "cortina,gemini-ethernet-port";
+ reg = <0x6000c000 0x2000>, /* Port 1 DMA/TOE */
+ <0x6000e000 0x2000>; /* Port 1 GMAC */
+ interrupt-parent = <&intcon>;
+ interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
+ resets = <&syscon GEMINI_RESET_GMAC1>;
+ clocks = <&syscon GEMINI_CLK_GATE_GMAC1>;
+ clock-names = "PCLK";
+ phy-mode = "rgmii";
+ phy-handle = <&phy1>;
+ };
+};
--
2.14.3
^ permalink raw reply related
* [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt
From: Grygorii Strashko @ 2018-01-12 21:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180112210844.GF4821@atomide.com>
On 01/12/2018 03:08 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [180112 20:36]:
>> Hi
>>
>> On 01/10/2018 10:54 PM, Qi Hou wrote:
>>> When more than one GP timers are used as kernel system timers and the
>>> corresponding nodes in device-tree are marked with the same "disabled"
>>> property, then the "attr" field of the property will be initialized
>>> more than once as the property being added to sys file system via
>>> __of_add_property_sysfs().
>>>
>>> In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
>>> directly to the return value of safe_name(), without taking care of
>>> whether it's already a valid pointer to a memory block. If it is, its
>>> old value will always be overwritten by the new one and the memory block
>>> allocated before will a "ghost", then a kmemleak happened.
>>>
>>> That the same "disabled" property being added to different nodes of device
>>> tree would cause that kind of kmemleak overhead, at leat once.
>>>
>>> To fix it, allocate the property dynamically, and delete static one.
>>
>> Does it in sync with Keerthy's work [1]
>
> First fixes, then new stuff! Keerthy's work will have to
> wait for v4.17, we want that series sitting in Linux next
> for several weeks.
>
Np. just want to be sure every party is aware about each other's work
--
regards,
-grygorii
^ permalink raw reply
* PM regression in next
From: Mark Brown @ 2018-01-12 21:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180112210706.GE4821@atomide.com>
On Fri, Jan 12, 2018 at 01:07:06PM -0800, Tony Lindgren wrote:
> (twl4030_dummy_read [snd_soc_twl4030]) from [<bf1cc3b4>]
> (snd_soc_codec_drv_read+0x1c/0x28 [snd_soc_core])
> (snd_soc_codec_drv_read [snd_soc_core]) from [<bf1da1cc>]
> (snd_soc_dapm_new_widgets+0x29c/0x578 [snd_soc_core])
> (snd_soc_dapm_new_widgets [snd_soc_core]) from [<bf1d2f30>]
> (snd_soc_register_card+0xad0/0xe30 [snd_soc_core])
> (snd_soc_register_card [snd_soc_core]) from [<bf1e1850>]
> (devm_snd_soc_register_card+0x30/0x70 [snd_soc_core])
> (devm_snd_soc_register_card [snd_soc_core]) from [<bf234364>]
> (omap_twl4030_probe+0x100/0x1d0 [snd_soc_omap_twl4030])
> (omap_twl4030_probe [snd_soc_omap_twl4030]) from [<c0606660>]
> (platform_drv_probe+0x50/0xb0)
So does the read not happen otherwise? Or is it failing somehow and the
error getting ignored?
> So probably there are other asoc drivers broken too with these
> kind of patches until snd_soc_codec_drv_write() and
> snd_soc_codec_drv_read() are fixed?
I rather suspect someone would've noticed by now TBH - I suspect this is
more likely to be an issue with the rather baroque code that the TWL
drivers have possibly coupled with the whole multiple I2C devices issue.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180112/f7329e49/attachment.sig>
^ permalink raw reply
* Applied "spi: orion: Fix clock resource by adding an optional bus clock" to the spi tree
From: Mark Brown @ 2018-01-12 21:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180112104233.4952-1-gregory.clement@free-electrons.com>
The patch
spi: orion: Fix clock resource by adding an optional bus clock
has been applied to the spi tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 92ae112e477ac412decc3fdd5c1eeb6c90c266b4 Mon Sep 17 00:00:00 2001
From: Gregory CLEMENT <gregory.clement@free-electrons.com>
Date: Fri, 12 Jan 2018 11:42:33 +0100
Subject: [PATCH] spi: orion: Fix clock resource by adding an optional bus
clock
On Armada 7K/8K we need to explicitly enable the bus clock. The bus clock
is optional because not all the SoCs need them but at least for Armada
7K/8K it is actually mandatory.
The binding documentation is updating accordingly as well as mentioning
the mandatory clock which was also missing.
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Documentation/devicetree/bindings/spi/spi-orion.txt | 9 +++++++++
drivers/spi/spi-orion.c | 14 ++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/Documentation/devicetree/bindings/spi/spi-orion.txt b/Documentation/devicetree/bindings/spi/spi-orion.txt
index df8ec31f2f07..8434a65fc12a 100644
--- a/Documentation/devicetree/bindings/spi/spi-orion.txt
+++ b/Documentation/devicetree/bindings/spi/spi-orion.txt
@@ -18,8 +18,17 @@ Required properties:
The eight register sets following the control registers refer to
chip-select lines 0 through 7 respectively.
- cell-index : Which of multiple SPI controllers is this.
+- clocks : pointers to the reference clocks for this device, the first
+ one is the one used for the clock on the spi bus, the
+ second one is optional and is the clock used for the
+ functional part of the controller
+
Optional properties:
- interrupts : Is currently not used.
+- clock-names : names of used clocks, mandatory if the second clock is
+ used, the name must be "core", and "axi" (the latter
+ is only for Armada 7K/8K).
+
Example:
spi at 10600 {
diff --git a/drivers/spi/spi-orion.c b/drivers/spi/spi-orion.c
index 8974bb340b3a..482a0cf3b7aa 100644
--- a/drivers/spi/spi-orion.c
+++ b/drivers/spi/spi-orion.c
@@ -94,6 +94,7 @@ struct orion_spi {
struct spi_master *master;
void __iomem *base;
struct clk *clk;
+ struct clk *axi_clk;
const struct orion_spi_dev *devdata;
struct orion_direct_acc direct_access[ORION_NUM_CHIPSELECTS];
@@ -634,6 +635,14 @@ static int orion_spi_probe(struct platform_device *pdev)
if (status)
goto out;
+ /* The following clock is only used by some SoCs */
+ spi->axi_clk = devm_clk_get(&pdev->dev, "axi");
+ if (IS_ERR(spi->axi_clk) &&
+ PTR_ERR(spi->axi_clk) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ if (!IS_ERR(spi->axi_clk))
+ clk_prepare_enable(spi->axi_clk);
+
tclk_hz = clk_get_rate(spi->clk);
/*
@@ -725,6 +734,7 @@ static int orion_spi_probe(struct platform_device *pdev)
out_rel_pm:
pm_runtime_disable(&pdev->dev);
out_rel_clk:
+ clk_disable_unprepare(spi->axi_clk);
clk_disable_unprepare(spi->clk);
out:
spi_master_put(master);
@@ -738,6 +748,7 @@ static int orion_spi_remove(struct platform_device *pdev)
struct orion_spi *spi = spi_master_get_devdata(master);
pm_runtime_get_sync(&pdev->dev);
+ clk_disable_unprepare(spi->axi_clk);
clk_disable_unprepare(spi->clk);
spi_unregister_master(master);
@@ -754,6 +765,7 @@ static int orion_spi_runtime_suspend(struct device *dev)
struct spi_master *master = dev_get_drvdata(dev);
struct orion_spi *spi = spi_master_get_devdata(master);
+ clk_disable_unprepare(spi->axi_clk);
clk_disable_unprepare(spi->clk);
return 0;
}
@@ -763,6 +775,8 @@ static int orion_spi_runtime_resume(struct device *dev)
struct spi_master *master = dev_get_drvdata(dev);
struct orion_spi *spi = spi_master_get_devdata(master);
+ if (!IS_ERR(spi->axi_clk))
+ clk_prepare_enable(spi->axi_clk);
return clk_prepare_enable(spi->clk);
}
#endif
--
2.15.1
^ permalink raw reply related
* [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt
From: Tony Lindgren @ 2018-01-12 21:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7dd12934-862c-c7ad-5f43-43e4c6b08199@ti.com>
* Grygorii Strashko <grygorii.strashko@ti.com> [180112 20:36]:
> Hi
>
> On 01/10/2018 10:54 PM, Qi Hou wrote:
> > When more than one GP timers are used as kernel system timers and the
> > corresponding nodes in device-tree are marked with the same "disabled"
> > property, then the "attr" field of the property will be initialized
> > more than once as the property being added to sys file system via
> > __of_add_property_sysfs().
> >
> > In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
> > directly to the return value of safe_name(), without taking care of
> > whether it's already a valid pointer to a memory block. If it is, its
> > old value will always be overwritten by the new one and the memory block
> > allocated before will a "ghost", then a kmemleak happened.
> >
> > That the same "disabled" property being added to different nodes of device
> > tree would cause that kind of kmemleak overhead, at leat once.
> >
> > To fix it, allocate the property dynamically, and delete static one.
>
> Does it in sync with Keerthy's work [1]
First fixes, then new stuff! Keerthy's work will have to
wait for v4.17, we want that series sitting in Linux next
for several weeks.
Regards,
Tony
> [1] https://www.spinics.net/lists/linux-omap/msg141100.html
^ permalink raw reply
* [PATCH 2/2] soc: bcm: brcmstb: Be multi-platform compatible
From: Florian Fainelli @ 2018-01-12 21:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180112210815.9684-1-f.fainelli@gmail.com>
From: Thierry Reding <treding@nvidia.com>
We were making a bunch of wrong assumptions that turned out to blow out
on non-Broadcom STB platforms:
- we would return -ENODEV from brcmstb_soc_device_early_init() if we
could not find the sun_top_ctrl device node, this is not an error
in the context of a multi-platform kernel
- we would still try to register the Broadcom STB SoC device, even if we
are not running on such a platform
While at it, also fix the sun_top_ctrl device_node leaks while we change
the flow of brcmstb_soc_device_init() and
brcmstb_soc_device_early_init().
Fixes: f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by: Thierry Reding <treding@nvidia.com>
[florian: Combine all of Thierry's patch in one go for easier review]
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/soc/bcm/brcmstb/common.c | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
index 781ada62d0a3..14185451901d 100644
--- a/drivers/soc/bcm/brcmstb/common.c
+++ b/drivers/soc/bcm/brcmstb/common.c
@@ -70,30 +70,49 @@ static int __init brcmstb_soc_device_early_init(void)
{
struct device_node *sun_top_ctrl;
void __iomem *sun_top_ctrl_base;
+ int ret = 0;
+ /* We could be on a multi-platform kernel, don't make this fatal but
+ * bail out early
+ */
sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
if (!sun_top_ctrl)
- return -ENODEV;
+ return ret;
sun_top_ctrl_base = of_iomap(sun_top_ctrl, 0);
- if (!sun_top_ctrl_base)
- return -ENODEV;
+ if (!sun_top_ctrl_base) {
+ ret = -ENODEV;
+ goto out;
+ }
family_id = readl(sun_top_ctrl_base);
product_id = readl(sun_top_ctrl_base + 0x4);
iounmap(sun_top_ctrl_base);
- return 0;
+out:
+ of_node_put(sun_top_ctrl);
+ return ret;
}
early_initcall(brcmstb_soc_device_early_init);
static int __init brcmstb_soc_device_init(void)
{
struct soc_device_attribute *soc_dev_attr;
+ struct device_node *sun_top_ctrl;
struct soc_device *soc_dev;
+ int ret = 0;
+
+ /* We could be on a multi-platform kernel, don't make this fatal but
+ * bail out early
+ */
+ sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+ if (!sun_top_ctrl)
+ return ret;
soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
- if (!soc_dev_attr)
- return -ENOMEM;
+ if (!soc_dev_attr) {
+ ret = -ENOMEM;
+ goto out;
+ }
soc_dev_attr->family = kasprintf(GFP_KERNEL, "%x",
family_id >> 28 ?
@@ -111,9 +130,10 @@ static int __init brcmstb_soc_device_init(void)
kfree(soc_dev_attr->soc_id);
kfree(soc_dev_attr->revision);
kfree(soc_dev_attr);
- return -ENOMEM;
+ ret = -ENOMEM;
}
-
- return 0;
+out:
+ of_node_put(sun_top_ctrl);
+ return ret;
}
arch_initcall(brcmstb_soc_device_init);
--
2.14.1
^ permalink raw reply related
* [PATCH 1/2] soc: brcmstb: biuctrl: exit without warning on non brcmstb platforms
From: Florian Fainelli @ 2018-01-12 21:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180112210815.9684-1-f.fainelli@gmail.com>
From: Sudeep Holla <sudeep.holla@arm.com>
Currently if this driver is included, we get the following warning
on any platforms irrespective of whether it's brcmstb platform or not.
"
brcmstb: biuctrl: missing BIU control node
brcmstb: biuctrl: MCP: Unable to disable write pairing!
"
This patch allows to exit early without any warning messages on non
brcmstb platforms as it's meaningless for them.
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: Gregory Fong <gregory.0xf0@gmail.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: bcm-kernel-feedback-list at broadcom.com
Fixes: f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
[florian: Add fixes tag, make initcall non fatal]
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/soc/bcm/brcmstb/biuctrl.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/soc/bcm/brcmstb/biuctrl.c b/drivers/soc/bcm/brcmstb/biuctrl.c
index 2b23ae7b5e9b..6d89ebf13b8a 100644
--- a/drivers/soc/bcm/brcmstb/biuctrl.c
+++ b/drivers/soc/bcm/brcmstb/biuctrl.c
@@ -162,17 +162,11 @@ static void __init mcp_b53_set(void)
cbc_writel(reg, CPU_WRITEBACK_CTRL_REG);
}
-static int __init setup_hifcpubiuctrl_regs(void)
+static int __init setup_hifcpubiuctrl_regs(struct device_node *np)
{
- struct device_node *np, *cpu_dn;
+ struct device_node *cpu_dn;
int ret = 0;
- np = of_find_compatible_node(NULL, NULL, "brcm,brcmstb-cpu-biu-ctrl");
- if (!np) {
- pr_err("missing BIU control node\n");
- return -ENODEV;
- }
-
cpubiuctrl_base = of_iomap(np, 0);
if (!cpubiuctrl_base) {
pr_err("failed to remap BIU control base\n");
@@ -242,9 +236,17 @@ static struct syscore_ops brcmstb_cpu_credit_syscore_ops = {
static int __init brcmstb_biuctrl_init(void)
{
+ struct device_node *np;
int ret;
- setup_hifcpubiuctrl_regs();
+ /* We might be running on a multi-platform kernel, don't make this a
+ * fatal error, just bail out early
+ */
+ np = of_find_compatible_node(NULL, NULL, "brcm,brcmstb-cpu-biu-ctrl");
+ if (!np)
+ return 0;
+
+ setup_hifcpubiuctrl_regs(np);
ret = mcp_write_pairing_set();
if (ret) {
--
2.14.1
^ permalink raw reply related
* [PATCH 0/2] soc: brcmstb: Fix multi-platform issues
From: Florian Fainelli @ 2018-01-12 21:08 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
This incorporates Sudeep's and Thierry's changes and makes minor tweaks
such that we don't return errors from initcalls even if we are not
running on Broadcom STB platforms.
I kept your authorship because you did 99% of the work, and I just added a
proper Fixes: tag to the first patch and combined Thierry's 4 patches into one.
This has now been properly tested by booting a QEMU ARM virtual machine to verify
we are not seeing BRCMSTB related messages anymore.
Again, sorry for missing the multi-platform kernel case!
Thank you!
Sudeep Holla (1):
soc: brcmstb: biuctrl: exit without warning on non brcmstb platforms
Thierry Reding (1):
soc: bcm: brcmstb: Be multi-platform compatible
drivers/soc/bcm/brcmstb/biuctrl.c | 20 +++++++++++---------
drivers/soc/bcm/brcmstb/common.c | 38 +++++++++++++++++++++++++++++---------
2 files changed, 40 insertions(+), 18 deletions(-)
--
2.14.1
^ permalink raw reply
* PM regression in next
From: Tony Lindgren @ 2018-01-12 21:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180112191251.GE21458@sirena.org.uk>
* Mark Brown <broonie@kernel.org> [180112 19:13]:
> On Fri, Jan 12, 2018 at 11:00:46AM -0800, Tony Lindgren wrote:
>
> > It's commit 3bb0f7c31b1a ("ASoC: don't use snd_soc_write/read
> > on twl4030"). And that is for the PMIC on my test system, so
> > adding Kuninori and Mark to the thread :)
>
> > Kuninori, it seems that commit 3bb0f7c31b1a causes higher
> > power consumption on an idle system on omap3 using twl4030.
> > Reverting 3bb0f7c31b1a makes things behave again. My guess
> > is that twl4030_read does not do the same as snd_soc_read
> > in the driver?
>
> As far as I can tell it should end up boiling down to the same thing but
> I didn't follow through in detail, they should both bottom out in
> twl_i2c_read_u8() if they hit hardware - all snd_soc_read() did was call
> twl4030_read(), the patch just removes the indirection through assigning
> the pointer.
>
> Could you try deleting the attempt to read from the cache in
> twl4030_read() and always go to hardware?
Thanks I tried that, but that's not it.
Tturns out just adding back .read = twl4030_read fixes it..
I added a dummy function for read and am now seeing a bunch
of reads that now don't happen:
(twl4030_dummy_read [snd_soc_twl4030]) from [<bf1cc3b4>]
(snd_soc_codec_drv_read+0x1c/0x28 [snd_soc_core])
(snd_soc_codec_drv_read [snd_soc_core]) from [<bf1da1cc>]
(snd_soc_dapm_new_widgets+0x29c/0x578 [snd_soc_core])
(snd_soc_dapm_new_widgets [snd_soc_core]) from [<bf1d2f30>]
(snd_soc_register_card+0xad0/0xe30 [snd_soc_core])
(snd_soc_register_card [snd_soc_core]) from [<bf1e1850>]
(devm_snd_soc_register_card+0x30/0x70 [snd_soc_core])
(devm_snd_soc_register_card [snd_soc_core]) from [<bf234364>]
(omap_twl4030_probe+0x100/0x1d0 [snd_soc_omap_twl4030])
(omap_twl4030_probe [snd_soc_omap_twl4030]) from [<c0606660>]
(platform_drv_probe+0x50/0xb0)
So probably there are other asoc drivers broken too with these
kind of patches until snd_soc_codec_drv_write() and
snd_soc_codec_drv_read() are fixed?
Regards,
Tony
^ permalink raw reply
* [PATCH 1/3] arm: spear600: Add missing interrupt-parent of rtc
From: Rob Herring @ 2018-01-12 20:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOesGMgcf9FovbzhPkKnrG4ETRFP3dsSJnTYNfp1wECMFeFOGA@mail.gmail.com>
On Fri, Jan 12, 2018 at 12:23 PM, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Jan 12, 2018 at 12:56 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Fri, Jan 12, 2018 at 4:23 AM, Olof Johansson <olof@lixom.net> wrote:
>>> On Thu, Jan 11, 2018 at 7:22 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>>> On 11-01-18, 18:07, Olof Johansson wrote:
>>>>> On Thu, Jan 11, 2018 at 11:28:51AM +0530, Viresh Kumar wrote:
>>>>> > The interrupt-parent of rtc was missing, add it.
>>>>> >
>>>>> > Fixes: 8113ba917dfa ("ARM: SPEAr: DT: Update device nodes")
>>>>> > Cc: stable at vger.kernel.org # v3.8+
>>>>> > Reported-by: Arnd Bergmann <arnd@arndb.de>
>>>>> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>>>>>
>>>>> Applied to next/dt. Is stable really needed on this? It's been broken since
>>>>> pretty much forever, and nobody has complained... :)
>>>>
>>>> Not sure. Just thought it may be useful for someone somewhere :)
>>>
>>> Ok. Left the tags there, but didn't merge into fixes since we're late
>>> in -rc and this didn't seem critical at this time.
>>
>> My plan was to have these in the fixes branch in the hope of making
>> it to a clean build for 4.15 after all, they fix warnings that got introduced
>> by the updated dtc checks in 4.15-rc1.
>>
>> We are getting fairly close, but it seems we still miss a few, so we
>> might as well give up at this point. The remaining fixes should be easy
>> to backport into v4.15.y if we decide to do it, of further back even.
>> For v4.14 and before, the in-kernel copy of dtc won't warn, but mainline
>> dtc will.
>>
>> Greg, let me know your thoughts on this for the upcoming 4.15.y
>> release. We had hundreds of dtc warnings in 4.15-rc1, many of them
>> about important bugs, now we're down to a couple of warnings
>> for platforms we don't care about much, and I expect the last of
>> these fixes to land in 4.16-rc1 or maybe -rc2. Shall we backport
>> them all to get a clean 4.15.y release?
>
> I think it makes more sense to disable the warnings than to backport a
> bunch of warning fixes this late. The code is working, has worked for
> a long time it's just that Rob enabled the warnings by default. We can
> keep them enabled for 4.16.
In some cases "working" was what's in the DT is unused by the kernel
because the DT is broken. That's why this round was not off by
default.
It looks to me to be somewhere less than 5 fixes remaining (BTW, why
is there no arm32 allmodconfig build on kernelci.org? That or
allyesconfig are the only ways to build all dtbs). It would also be an
exception to the the stable process because that patch would not be in
Linus' tree.
>> Note: there was at least one dtc warning fix that caused a serious
>> regression in code that relied on a device probe to fail because of
>> an invalid node (a fix is still in the works for 4.15), though generally
>> the fixes are really harmless and can only make things better.
>
> Exactly why picking up warning fixes this late is probably not a great idea.
Applying them now for 4.15 would be late, but tagging them as fixes in
4.16 would not be late (other than the normal problem that things get
applied to stable before sufficient testing in master).
I have more dtc checks in the works (nothing for 4.16 :) ). I'd like
the process to work better. I'm not going to fix all the warnings. I
don't think Arnd should either. Turning them off by default hasn't
worked great either. For some, I'm not sure we can ever get to warning
free, but I'd like new stuff to have warnings enabled and no one
builds with W=1. We could put together tooling to just show new
warnings, but someone has to run it and enforce it. I could stick dtc
updates into linux-next for multiple cycles before sending to Linus.
Rob
^ permalink raw reply
* [PATCH v2 04/12] drm/bridge/synopsys: dw-hdmi: Export some PHY related functions
From: Jernej Škrabec @ 2018-01-12 20:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180110192512.19684-5-jernej.skrabec@siol.net>
Hi all,
Dne sreda, 10. januar 2018 ob 20:25:04 CET je Jernej Skrabec napisal(a):
> Parts of PHY code could be useful also for custom PHYs. For example,
> Allwinner A83T has custom PHY which is probably Synopsys gen2 PHY
> with few additional memory mapped registers, so most of the Synopsys PHY
> related code could be reused.
>
> Functions exported here are actually not specific to Synopsys PHYs but
> to DWC HDMI controller PHY interface. This means that even if the PHY is
> completely custom, i.e. not designed by Synopsys, exported functions can
> be useful.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 44
> +++++++++++++++++++++---------- include/drm/bridge/dw_hdmi.h |
> 11 ++++++++
> 2 files changed, 41 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> 7ca14d7325b5..7d80f4b56683 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1037,19 +1037,21 @@ static void dw_hdmi_phy_enable_svsret(struct dw_hdmi
> *hdmi, u8 enable) HDMI_PHY_CONF0_SVSRET_MASK);
> }
>
> -static void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable)
> +void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable)
> {
> hdmi_mask_writeb(hdmi, enable, HDMI_PHY_CONF0,
> HDMI_PHY_CONF0_GEN2_PDDQ_OFFSET,
> HDMI_PHY_CONF0_GEN2_PDDQ_MASK);
> }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_pddq);
>
> -static void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable)
> +void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable)
> {
> hdmi_mask_writeb(hdmi, enable, HDMI_PHY_CONF0,
> HDMI_PHY_CONF0_GEN2_TXPWRON_OFFSET,
> HDMI_PHY_CONF0_GEN2_TXPWRON_MASK);
> }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_txpwron);
>
> static void dw_hdmi_phy_sel_data_en_pol(struct dw_hdmi *hdmi, u8 enable)
> {
> @@ -1065,6 +1067,22 @@ static void dw_hdmi_phy_sel_interface_control(struct
> dw_hdmi *hdmi, u8 enable) HDMI_PHY_CONF0_SELDIPIF_MASK);
> }
>
> +void dw_hdmi_phy_reset(struct dw_hdmi *hdmi)
> +{
> + /* PHY reset. The reset signal is active high on Gen2 PHYs. */
> + hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_PHYRSTZ, HDMI_MC_PHYRSTZ);
> + hdmi_writeb(hdmi, 0, HDMI_MC_PHYRSTZ);
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_reset);
I just noticed that meson dw hdmi driver uses function with the same name,
which break compilation. Is it ok if I rename meson specific reset to
meson_dw_hdmi_phy_reset()?
Best regards,
Jernej
> +
> +void dw_hdmi_phy_i2c_set_addr(struct dw_hdmi *hdmi, u8 address)
> +{
> + hdmi_phy_test_clear(hdmi, 1);
> + hdmi_writeb(hdmi, address, HDMI_PHY_I2CM_SLAVE_ADDR);
> + hdmi_phy_test_clear(hdmi, 0);
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_i2c_set_addr);
> +
> static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
> {
> const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
> @@ -1203,16 +1221,11 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
> if (phy->has_svsret)
> dw_hdmi_phy_enable_svsret(hdmi, 1);
>
> - /* PHY reset. The reset signal is active high on Gen2 PHYs. */
> - hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_PHYRSTZ, HDMI_MC_PHYRSTZ);
> - hdmi_writeb(hdmi, 0, HDMI_MC_PHYRSTZ);
> + dw_hdmi_phy_reset(hdmi);
>
> hdmi_writeb(hdmi, HDMI_MC_HEACPHY_RST_ASSERT, HDMI_MC_HEACPHY_RST);
>
> - hdmi_phy_test_clear(hdmi, 1);
> - hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
> - HDMI_PHY_I2CM_SLAVE_ADDR);
> - hdmi_phy_test_clear(hdmi, 0);
> + dw_hdmi_phy_i2c_set_addr(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2);
>
> /* Write to the PHY as configured by the platform */
> if (pdata->configure_phy)
> @@ -1251,15 +1264,16 @@ static void dw_hdmi_phy_disable(struct dw_hdmi
> *hdmi, void *data) dw_hdmi_phy_power_off(hdmi);
> }
>
> -static enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> - void *data)
> +enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> + void *data)
> {
> return hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD ?
> connector_status_connected : connector_status_disconnected;
> }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_read_hpd);
>
> -static void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> - bool force, bool disabled, bool rxsense)
> +void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> + bool force, bool disabled, bool rxsense)
> {
> u8 old_mask = hdmi->phy_mask;
>
> @@ -1271,8 +1285,9 @@ static void dw_hdmi_phy_update_hpd(struct dw_hdmi
> *hdmi, void *data, if (old_mask != hdmi->phy_mask)
> hdmi_writeb(hdmi, hdmi->phy_mask, HDMI_PHY_MASK0);
> }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_update_hpd);
>
> -static void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
> +void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
> {
> /*
> * Configure the PHY RX SENSE and HPD interrupts polarities and clear
> @@ -1291,6 +1306,7 @@ static void dw_hdmi_phy_setup_hpd(struct dw_hdmi
> *hdmi, void *data) hdmi_writeb(hdmi, ~(HDMI_IH_PHY_STAT0_HPD |
> HDMI_IH_PHY_STAT0_RX_SENSE), HDMI_IH_MUTE_PHY_STAT0);
> }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_setup_hpd);
>
> static const struct dw_hdmi_phy_ops dw_hdmi_synopsys_phy_ops = {
> .init = dw_hdmi_phy_init,
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 182f83283e24..4a35e5065f6f 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -157,7 +157,18 @@ void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
> void dw_hdmi_audio_disable(struct dw_hdmi *hdmi);
>
> /* PHY configuration */
> +void dw_hdmi_phy_i2c_set_addr(struct dw_hdmi *hdmi, u8 address);
> void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
> unsigned char addr);
>
> +enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> + void *data);
> +void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> + bool force, bool disabled, bool rxsense);
> +void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);
> +
> +void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable);
> +void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable);
> +void dw_hdmi_phy_reset(struct dw_hdmi *hdmi);
> +
> #endif /* __IMX_HDMI_H__ */
> --
> 2.15.1
^ permalink raw reply
* [PATCH, v3] arm: omap2: timer: fix a kmemleak caused in omap_get_timer_dt
From: Grygorii Strashko @ 2018-01-12 20:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1515646483-24199-1-git-send-email-qi.hou@windriver.com>
Hi
On 01/10/2018 10:54 PM, Qi Hou wrote:
> When more than one GP timers are used as kernel system timers and the
> corresponding nodes in device-tree are marked with the same "disabled"
> property, then the "attr" field of the property will be initialized
> more than once as the property being added to sys file system via
> __of_add_property_sysfs().
>
> In __of_add_property_sysfs(), the "name" field of pp->attr.attr is set
> directly to the return value of safe_name(), without taking care of
> whether it's already a valid pointer to a memory block. If it is, its
> old value will always be overwritten by the new one and the memory block
> allocated before will a "ghost", then a kmemleak happened.
>
> That the same "disabled" property being added to different nodes of device
> tree would cause that kind of kmemleak overhead, at leat once.
>
> To fix it, allocate the property dynamically, and delete static one.
Does it in sync with Keerthy's work [1]
[1] https://www.spinics.net/lists/linux-omap/msg141100.html
>
> Signed-off-by: Qi Hou <qi.hou@windriver.com>
> ---
> arch/arm/mach-omap2/timer.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
--
regards,
-grygorii
^ permalink raw reply
* [bug report] perf: ARM DynamIQ Shared Unit PMU support
From: Dan Carpenter @ 2018-01-12 20:13 UTC (permalink / raw)
To: linux-arm-kernel
Hello Suzuki K Poulose,
The patch 7520fa99246d: "perf: ARM DynamIQ Shared Unit PMU support"
from Jan 2, 2018, leads to the following static checker warning:
drivers/perf/arm_dsu_pmu.c:680 dsu_pmu_init_pmu()
warn: impossible condition '(dsu_pmu->num_counters == -1) => (0-255 == (-1))'
drivers/perf/arm_dsu_pmu.c
674 /*
675 * dsu_pmu_init_pmu: Initialise the DSU PMU configurations if
676 * we haven't done it already.
677 */
678 static void dsu_pmu_init_pmu(struct dsu_pmu *dsu_pmu)
679 {
680 if (dsu_pmu->num_counters == -1)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
A u8 variable can't ever be -1.
681 dsu_pmu_probe_pmu(dsu_pmu);
682 /* Reset the interrupt overflow mask */
683 dsu_pmu_get_reset_overflow();
684 }
regards,
dan carpenter
^ permalink raw reply
* [PATCH] phy: work around 'phys' references to usb-nop-xceiv devices
From: Rob Herring @ 2018-01-12 19:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180112101223.3661181-1-arnd@arndb.de>
On Fri, Jan 12, 2018 at 4:12 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> Stefan Wahren reports a problem with a warning fix that was merged
> for v4.15: we had lots of device nodes with a 'phys' property pointing
> to a device node that is not compliant with the binding documented in
> Documentation/devicetree/bindings/phy/phy-bindings.txt
>
> This generally works because USB HCD drivers that support both the generic
> phy subsystem and the older usb-phy subsystem ignore most errors from
> phy_get() and related calls and then use the usb-phy driver instead.
>
> However, it turns out that making the usb-nop-xceiv device compatible with
> the generic-phy binding changes the phy_get() return code from -EINVAL to
> -EPROBE_DEFER, and the dwc2 usb controller driver for bcm2835 now returns
> -EPROBE_DEFER from its probe function rather than ignoring the failure,
> breaking all USB support on raspberry-pi when CONFIG_GENERIC_PHY is
> enabled. The same code is used in the dwc3 driver and the usb_add_hcd()
> function, so a reasonable assumption would be that many other platforms
> are affected as well.
>
> I have reviewed all the related patches and concluded that "usb-nop-xceiv"
> is the only USB phy that is affected by the change, and since it is by far
> the most commonly referenced phy, all the other USB phy drivers appear
> to be used in ways that are are either safe in DT (they don't use the
> 'phys' property), or in the driver (they already ignore -EPROBE_DEFER
> from generic-phy when usb-phy is available).
>
> To work around the problem, this adds a special case to _of_phy_get()
> so we ignore any PHY node that is compatible with "usb-nop-xceiv",
> as we know that this can never load no matter how much we defer. In the
> future, we might implement a generic-phy driver for "usb-nop-xceiv"
> and then remove this workaround.
>
> Since we generally want older kernels to also want to work with the
> fixed devicetree files, it would be good to backport the patch into
> stable kernels as well (3.13+ are possibly affected), even though they
> don't contain any of the patches that may have caused regressions.
>
> Fixes: 014d6da6cb25 ARM: dts: bcm283x: Fix DTC warnings about missing phy-cells
> Fixes: c5bbf358b790 arm: dts: nspire: Add missing #phy-cells to usb-nop-xceiv
> Fixes: 44e5dced2ef6 arm: dts: marvell: Add missing #phy-cells to usb-nop-xceiv
> Fixes: f568f6f554b8 ARM: dts: omap: Add missing #phy-cells to usb-nop-xceiv
> Fixes: d745d5f277bf ARM: dts: imx51-zii-rdu1: Add missing #phy-cells to usb-nop-xceiv
> Fixes: 915fbe59cbf2 ARM: dts: imx: Add missing #phy-cells to usb-nop-xceiv
> Link: https://marc.info/?l=linux-usb&m=151518314314753&w=2
> Link: https://patchwork.kernel.org/patch/10158145/
> Cc: stable at vger.kernel.org
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Eric Anholt <eric@anholt.net>
> Tested-by: Hans Verkuil <hans.verkuil@cisco.com>
> Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Hans tested the earlier version of this patch, I'd like one more
> confirmation from Hans or Stefan (or anyone else) that this version
> addresses the regression as well before this gets merged.
>
> Greg, can you pick this up into usb-linus for v4.15 once the fix
> has been confirmed, or should I merge it through arm-soc?
> ---
> drivers/phy/phy-core.c | 4 ++++
> 1 file changed, 4 insertions(+)
Thanks for fixing this.
Acked-by: Rob Herring <robh@kernel.org>
Rob
^ permalink raw reply
* [PATCH] usb: dwc2: Fix endless deferral probe
From: Arnd Bergmann @ 2018-01-12 19:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180112155107.39b9d765@vento.lan>
On Fri, Jan 12, 2018 at 6:51 PM, Mauro Carvalho Chehab
<mchehab@s-opensource.com> wrote:
> Em Fri, 12 Jan 2018 10:18:59 +0100
> Arnd Bergmann <arnd@arndb.de> escreveu:
>
>> On Fri, Jan 12, 2018 at 9:06 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>> > Am 12.01.2018 um 00:32 schrieb Arnd Bergmann:
>> >> On Wed, Jan 10, 2018 at 1:15 PM, Stefan Wahren <stefan.wahren@i2se.com>
>>
>> >> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> >> index b4964b067aec..93b55fb71d54 100644
>> >> --- a/drivers/phy/phy-core.c
>> >> +++ b/drivers/phy/phy-core.c
>> >> @@ -410,6 +410,10 @@ static struct phy *_of_phy_get(struct device_node
>> >> *np, int index)
>> >> if (ret)
>> >> return ERR_PTR(-ENODEV);
>> >>
>> >> + /* This phy type handled by the usb-phy subsystem for now */
>> >> + if (of_device_is_compatible(np, "usb-nop-xceiv"))
>> >> + return ERR_PTR(-ENODEV);
>> >> +
>> >> mutex_lock(&phy_provider_mutex);
>> >> phy_provider = of_phy_provider_lookup(args.np);
>> >> if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner))
>> >> {
>> >
>> >
>> > I tried this, but it doesn't work. "np" is the node of the USB controller,
>> > not of the phy?
>>
>> Correct, that was a really dumb mistake on my end, I'm glad I asked
>> you to try it first.
>>
>> I'll fix it up and send the right version with a proper changelog right away.
>
> I suspect you meant to do, instead:
>
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index b4964b067aec..584cc1469f7d 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -410,6 +410,10 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
> if (ret)
> return ERR_PTR(-ENODEV);
>
> + /* This phy type handled by the usb-phy subsystem for now */
> + if (of_device_is_compatible(args.np, "usb-nop-xceiv"))
> + return ERR_PTR(-ENODEV);
> +
> mutex_lock(&phy_provider_mutex);
> phy_provider = of_phy_provider_lookup(args.np);
> if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
>
>
> (e. g. args.np, instead of np).
>
> The above works for me on RPi3:
>
> $ uname -a
> Linux raspberrypi 4.15.0-rc7-mcc+ #28 SMP Fri Jan 12 12:35:19 EST 2018 armv7l GNU/Linux
>
> $ lsusb
> Bus 001 Device 005: ID 045e:0750 Microsoft Corp. Wired Keyboard 600
> Bus 001 Device 004: ID 0572:960c Conexant Systems (Rockwell), Inc. DVBSky S960C DVB-S2 tuner
> Bus 001 Device 003: ID 0424:ec00 Standard Microsystems Corp. SMSC9512/9514 Fast Ethernet Adapter
> Bus 001 Device 002: ID 0424:9514 Standard Microsystems Corp. SMC9514 Hub
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
>
> So, if you change np -> args.np, feel free to add my:
>
> Tested-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Thanks a lot for testing, args.np is what the version I sent today has after
Stefan pointed out the mistake earlier.
Arnd
^ permalink raw reply
* [PATCH v2 3/7] PCI: aardvark: set host and device to the same MAX payload size
From: Bjorn Helgaas @ 2018-01-12 19:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180112164644.5611fe3b@windsurf.lan>
On Fri, Jan 12, 2018 at 04:46:44PM +0100, Thomas Petazzoni wrote:
> On Fri, 12 Jan 2018 08:40:24 -0600, Bjorn Helgaas wrote:
> > The Device Control MPS field defaults to 128 bytes. Generic software
> > can only change that default if it knows that every element that might
> > receive a packet from the device can handle it. In this case, we have
> > no information about what the invisible Root Port can handle, so I
> > would argue that we cannot change MPS.
> >
> > In the lspci above, MPS is set to 256 bytes. If that was done by
> > firmware, it might be safe because it knows things about the Root Port
> > that Linux doesn't. But I don't think the Linux PCI core could set it
> > to 256.
>
> So you're suspecting that the firmware/bootloader has configured the
> MPS on the E1000E device to 256 bytes ?
I didn't word that very well. It looks like *something* set it to
256, but I don't know what. It's possible Linux did, but I think that
would be a bug and should be fixed. We'd have to instrument the code
or analyze it more closely than I have.
> Isn't it dangerous for the kernel to rely on the firmware/bootloader
> configuration ? Indeed, the firmware/bootloader might have configured
> MPS to X bytes on the endpoint, but when the kernel boots and
> initializes the PCIe controller, its sets the PCIe controller MPS to Y
> bytes, with Y > X.
>
> > ASPM L0s is similar. We should only enable L0s if we can tell that
> > both ends of the link support it. If there's no Root Port, we don't
> > have any ASPM capability information for the upstream end of the link,
> > so we shouldn't enable ASPM at all.
>
> Well, even without the Root Port, we are able to use the endpoint
> configuration space to figure out whether it supports L0s, and adjust
> the root complex configuration accordingly. This is what our patch is
> doing for MPS, and which could be done similarly for L0s, no?
Yes, this is where it would get machine-specific. I don't know how
that should be structured, or even whether it's really worthwhile. I
think the core should make it *work* with the least-common-denominator
approach, but I'm not convinced that a lot of effort should be put
into optimizing a topology that doesn't follow the spec. A driver
could do this outside the core, but I think it would be better to put
the effort into making the topology more standard.
Why exactly *doesn't* Aardvark expose the Root Port? I assume it does
actually exist, since there is actually a link leading to the slot,
and there has to be *something* at the upstream end of that link.
> > I had the impression that these patches were required for correct
> > functionality, not just to improve performance. But maybe I
> > misunderstood?
>
> I don't myself have the device that wasn't working, and that this patch
> got to work, so I can't double check myself. However, indeed, I was
> told that without this fix, some devices would not work.
>
> One question: is it valid/working to have the root complex configured
> with MPS = 128 bytes but the endpoint configured with MPS = 256 or 512
> bytes ? Or should the MPS value be strictly equal on both sides ?
Per PCIe r4.0, sec 2.2.2, a device cannot transmit a TLP with a
payload larger than its MPS. A device receiving a TLP with a payload
larger than its MPS must treat the TLP as malformed.
I think that means MPS really should be set the same on both sides so
the device can do both DMA reads and writes safely.
> Depending on your answer, there are two options:
>
> - It is a valid situation to have a root complex MPS lower than the
> endpoint MPS. In this case, we could for now simply unconditionally
> set the MPS to 128 bytes in the root complex, as a fix to get all
> devices working. And then separately, work on improving performance
> by increasing the MPS according to the endpoint capabilities.
>
> - It is not valid for the root complex MPS to be different than the
> endpoint MPS. In this case, then I don't see how we can do things
> differently than the proposed patch: we have to see what the
> endpoint MPS is, and adjust the root complex MPS accordingly.
> Indeed, the bootloader/firmware might have changed the endpoint MPS
> so that it is no longer the default of 128 bytes.
All devices are guaranteed to support MPS = 128 bytes, so if you set
the Root Port to that in the driver, we should be able to make the PCI
core leave (or set, if necessary) all devices with MPS = 128.
I think we should start with that first, then worry about performance
optimizations separately. I guess I'm still just shaking my head over
the invisible Root Port mystery. I think the other cases I know about
are related to virtualization, where I can sort of understand why the
Root Port is missing, but I don't think that's the situation here.
Bjorn
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox