* [RFC PATCH 5/5] of: overlay-mgr: add a detector for headers stored on a ds2431 eeprom over w1
From: Antoine Tenart @ 2016-10-27 13:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e839e92f-127b-efc3-b10f-37477d1c5d9a@suse.com>
Hello Matthias,
On Thu, Oct 27, 2016 at 11:19:14AM +0200, Matthias Brugger wrote:
> On 10/26/2016 04:57 PM, Antoine Tenart wrote:
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> > ---
>
> Please provide a commit message.
Sure. There are other modifications I'd like to do in the series if it
happens to be an use case for people. This patch is given as an example
of how we could implement this.
Antoine
--
Antoine T?nart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161027/109caa98/attachment-0001.sig>
^ permalink raw reply
* [RFC PATCH 0/5] Add an overlay manager to handle board capes
From: Rob Herring @ 2016-10-27 13:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161026145756.21689-1-antoine.tenart@free-electrons.com>
Please Cc the maintainers of drivers/of/.
+ Frank R, Hans, Dmitry S
On Wed, Oct 26, 2016 at 9:57 AM, Antoine Tenart
<antoine.tenart@free-electrons.com> wrote:
> Hi all,
>
> Many boards now come with dips and compatible capes; among others the
> C.H.I.P, or Beaglebones. All these boards have a kernel implementing an
> out-of-tree "cape manager" which is used to detected capes, retrieve
> their description and apply a corresponding overlay. This series is an
> attempt to start a discussion, with an implementation of such a manager
> which is somehow generic (i.e. formats or cape detectors can be added).
> Other use cases could make use of this manager to dynamically load dt
> overlays based on some input / hw presence.
I'd like to see an input source be the kernel command line and/or a DT
chosen property. Another overlay manager was proposed not to long
ago[1] as well. There's also the Allwinner tablet use case from Hans
where i2c devices are probed and detected. That's not using overlays
currently, but maybe could.
Another thing to consider is different sources of overlays. Besides in
the filesystem, overlays could be built into the kernel (already
supported), embedded in the dtb (as the other overlay mgr did) or we
could extend FDT format to append them.
> The proposed design is a library which can be used by detector drivers
> to parse headers and load the corresponding overlay. Helpers are
> provided for this purpose. The whole thing is divided into 3 entities:
>
> - The parser which is project-specific (to allow supporting headers
> already into the wild). It registers a function parsing an header's
> data and filling one or more strings which will be used to find
> matching dtbo on the fs.
>
> - The overlay manager helpers allowing to parse a header to retrieve
> the previously mentioned strings and to load a compatible overlay.
>
> - The detectors which are used to detect capes and get their description
> (to be parsed).
What about things like power has to be turned on first to detect
boards and read their ID? I think this needs to be tied into the
driver model. Though, don't go sticking cape mgr nodes into DT. Maybe
a driver gets bound to a connector node, but we've got to sort out
connector bindings first.
> An example of parser and detector is given, compatible with what's done
> for the C.H.I.P. As the w1 framework is really bad (and we should
> probably do something about that) the detector code is far from being
> perfect; but that's not related to what we try to achieve here.
>
> The actual implementation has a limitation: the detectors cannot be
> built-in the kernel image as they would likely detect capes at boot time
> but will fail to get their corresponding dt overlays as the fs isn't
> mounted yet. The only case this can work is when dt overlays are
> built-in firmwares. This isn't an issue for the C.H.I.P. use case right
> now. There was a discussion about making an helper to wait for the
> rootfs to be mount but the answer was "this is the driver's problem".
I thought there are firmware loading calls that will wait. I think
this all needs to work asynchronously both for firmware loading and
because w1 is really slow.
> I'd like to get comments, specifically from people using custom cape
> managers, to see if this could fill their needs (with I guess some
> modifications).
Having 2 would certainly give a better sense this is generic.
Rob
[1] https://patchwork.ozlabs.org/patch/667805/
^ permalink raw reply
* [PATCH v2 2/4] dt-bindings: Add TI SCI PM Domains
From: Dave Gerlach @ 2016-10-27 13:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL_JsqJvm8+L0-pFQRQGYhvSzvqubWsBp8Q5kU-BPSiDmMau0A@mail.gmail.com>
+Jon
On 10/26/2016 04:59 PM, Rob Herring wrote:
> On Mon, Oct 24, 2016 at 12:00 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>> Dave Gerlach <d-gerlach@ti.com> writes:
>>
>>> Hi,
>>> On 10/21/2016 01:48 PM, Kevin Hilman wrote:
>>>> Dave Gerlach <d-gerlach@ti.com> writes:
>>>>
>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>>> control device power states.
>>>>>
>>>>> Also, provide macros representing each device index as understood
>>>>> by TI SCI to be used in the device node power-domain references.
>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>
>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>> ---
>>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 54 +++++++++++++
>>>>> MAINTAINERS | 2 +
>>>>> include/dt-bindings/genpd/k2g.h | 90 ++++++++++++++++++++++
>>>>> 3 files changed, 146 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>> create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>> new file mode 100644
>>>>> index 000000000000..32f38a349656
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>> @@ -0,0 +1,54 @@
>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>> +---------------------------------------------
>>>>> +
>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that is
>>>>> +responsible for controlling the state of the IPs that are present.
>>>>> +Communication between the host processor running an OS and the system
>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm domain
>>>>> +implementation plugs into the generic pm domain framework and makes use of
>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>> +
>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>> +
>>>>> +PM Domain Node
>>>>> +==============
>>>>> +The PM domain node represents the global PM domain managed by the PMMC,
>>>>> +which in this case is the single implementation as documented by the generic
>>>>> +PM domain bindings in Documentation/devicetree/bindings/power/power_domain.txt.
>>>>> +
>>>>> +Required Properties:
>>>>> +--------------------
>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>> +- #power-domain-cells: Must be 0.
>>>>> +- ti,sci: Phandle to the TI SCI device to use for managing the devices.
>>>>>
>>>>> +Example:
>>>>> +--------------------
>>>>> +k2g_pds: k2g_pds {
>>>>
>>>> should use generic name like "power-contoller", e.g. k2g_pds: power-controller
>>>
>>> Ok, that makes more sense.
>>>
>>>>
>>>>> + compatible = "ti,sci-pm-domain";
>>>>> + #power-domain-cells = <0>;
>>>>> + ti,sci = <&pmmc>;
>>>>> +};
>>>>> +
>>>>> +PM Domain Consumers
>>>>> +===================
>>>>> +Hardware blocks that require SCI control over their state must provide
>>>>> +a reference to the sci-pm-domain they are part of and a unique device
>>>>> +specific ID that identifies the device.
>>>>> +
>>>>> +Required Properties:
>>>>> +--------------------
>>>>> +- power-domains: phandle pointing to the corresponding PM domain node.
>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI to
>>>>> + be used for device control.
>>>>
>>>> This ID doesn't look right.
>>>>
>>>> Why not use #power-domain-cells = <1> and pass the index in the DT? ...
>
> Exactly. ti,sci-id is a NAK for me.
I was told not to use the onecell during v1 discussion. I agree this would be
ideal but I cannot due to what the bindings represent, the phandle parameter is
an index into a list of genpds, whereas we need an actual ID number we can use
and I do not have the ability to get that from the phandle.
@Ulf/Jon, is there any hope of bringing back custom xlate functions for genpd
providers? I don't have a good background on why it was even removed. I can
maintain a single genpd for all devices but I need a way to parse this ID,
whether it's from a separate property or a phandle. It is locked now to indexing
into a list of genpds but I need additional per device information for devices
bound to a genpd and I need either a custom parameter or the ability to parse
the phandle myself.
>
>>>>
>>>>> +See dt-bindings/genpd/k2g.h for the list of valid identifiers for k2g.
>>>>> +
>>>>> +Example:
>>>>> +--------------------
>>>>> +uart0: serial at 02530c00 {
>>>>> + compatible = "ns16550a";
>>>>> + ...
>>>>> + power-domains = <&k2g_pds>;
>>>>> + ti,sci-id = <K2G_DEV_UART0>;
>>>>
>>>> ... like this:
>>>>
>>>> power-domains = <&k2g_pds K2G_DEV_UART0>;
>>>
>>> That's how I did it in version one actually. I was able to define my
>>> own xlate function to parse the phandle and get that index, but Ulf
>>> pointed me to this series by Jon Hunter [1] that simplified genpd
>>> providers and dropped the concept of adding your own xlate. This locks
>>> the onecell approach to using a fixed static array of genpds that get
>>> indexed into (without passing the index to the provider, just the
>>> genpd that's looked up), which doesn't fit our usecase, as we don't
>>> want a 1 to 1 genpd to device mapping based on the comments provided
>>> in v1. Now we just use the genpd device attach/detach hooks to parse
>>> the sci-id and then use it in the genpd device start/stop hooks.
>
> I have no idea what any of this means. All sounds like driver
> architecture, not anything to do with bindings.
This was a response to Kevin, not part of binding description.
>
>>
>> Ah, right. I remember now. This approach allows you to use a single
>> genpd as discussed earlier.
>>
>> Makes sense now, suggestion retracted.
>
> IIRC, the bindings in Jon's case had a node for each domain and didn't
> need any additional property.
Yes but we only have one domain and index into it, not into a list of domains,
so the additional property is solving a different problem.
Regards,
Dave
>
> Rob
>
^ permalink raw reply
* [PATCH] i2c: rk3x: Give the tuning value 0 during rk3x_i2c_v0_calc_timings
From: Wolfram Sang @ 2016-10-27 13:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477125822-30644-1-git-send-email-david.wu@rock-chips.com>
On Sat, Oct 22, 2016 at 04:43:42PM +0800, David Wu wrote:
> We found a bug that i2c transfer sometimes failed on 3066a board with
> stabel-4.8, the con register would be updated by uninitialized tuning
> value, it made the i2c transfer failed.
>
> So give the tuning value to be zero during rk3x_i2c_v0_calc_timings.
>
> Signed-off-by: David Wu <david.wu@rock-chips.com>
What I missed to say in my review:
Please use a subject line that describes WHY the change is needed not so
much WHAT is done. Like: "fix missing initialization causing boot
problems"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161027/c2981e1f/attachment.sig>
^ permalink raw reply
* [PATCH V6 4/4] dts/imx6q-b850v3: Use GE B850v3 LVDS/DP++ Bridge
From: Peter Senna Tschudin @ 2016-10-27 13:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1477572814.git.peter.senna@collabora.com>
Configures the GE B850v3 LVDS/DP++ bridge on the dts file.
Cc: Martyn Welch <martyn.welch@collabora.co.uk>
Cc: Martin Donnelly <martin.donnelly@ge.com>
Cc: Javier Martinez Canillas <javier@dowhile0.org>
Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
arch/arm/boot/dts/imx6q-b850v3.dts | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-b850v3.dts b/arch/arm/boot/dts/imx6q-b850v3.dts
index 167f744..8db3bf2 100644
--- a/arch/arm/boot/dts/imx6q-b850v3.dts
+++ b/arch/arm/boot/dts/imx6q-b850v3.dts
@@ -72,6 +72,13 @@
fsl,data-mapping = "spwg";
fsl,data-width = <24>;
status = "okay";
+
+ port at 4 {
+ reg = <4>;
+ lvds0_out: endpoint {
+ remote-endpoint = <&b850v3_lvds_dp_bridge_in>;
+ };
+ };
};
};
@@ -142,3 +149,26 @@
reg = <0x4a>;
};
};
+
+&mux2_i2c2 {
+ status = "okay";
+ clock-frequency = <100000>;
+
+ b850v3-lvds-dp-bridge at 73 {
+ compatible = "ge,b850v3-lvds-dp";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ reg = <0x73>;
+ interrupt-parent = <&gpio2>;
+ interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+
+ edid-reg = <0x72>;
+
+ port {
+ b850v3_lvds_dp_bridge_in: endpoint {
+ remote-endpoint = <&lvds0_out>;
+ };
+ };
+ };
+};
--
2.5.5
^ permalink raw reply related
* [PATCH V6 3/4] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge
From: Peter Senna Tschudin @ 2016-10-27 13:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1477572814.git.peter.senna@collabora.com>
Add a driver that create a drm_bridge and a drm_connector for the LVDS
to DP++ display bridge of the GE B850v3.
There are two physical bridges on the video signal pipeline: a
STDP4028(LVDS to DP) and a STDP2690(DP to DP++). The hardware and
firmware made it complicated for this binding to comprise two device
tree nodes, as the design goal is to configure both bridges based on
the LVDS signal, which leave the driver powerless to control the video
processing pipeline. The two bridges behaves as a single bridge, and
the driver is only needed for telling the host about EDID / HPD, and
for giving the host powers to ack interrupts. The video signal pipeline
is as follows:
Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
Cc: Martyn Welch <martyn.welch@collabora.co.uk>
Cc: Martin Donnelly <martin.donnelly@ge.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
CC: David Airlie <airlied@linux.ie>
CC: Thierry Reding <treding@nvidia.com>
CC: Thierry Reding <thierry.reding@gmail.com>
CC: Archit Taneja <architt@codeaurora.org>
Reviewed-by: Enric Balletbo <enric.balletbo@collabora.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
Changes from V5:
- Reworked interrupt handler initialization
- Removed useless calls to: drm_connector_register(),
drm_helper_hpd_irq_event(), and drm_bridge_enable()
Changes from V4:
- Renamed the i2c_driver.name from "ge,b850v3-lvds-dp" to "b850v3-lvds-dp" to
remove the comma from the driver name
Changes from V3:
- 3/4 instead of 4/5
- Tested on next-20160804
Changes from V2:
- Made it atomic to be applied on next-20160729 on top of Liu Ying changes
that made imx-ldb atomic
Changes from V1:
- New commit message
- Removed 3 empty entry points
- Removed memory leak from ge_b850v3_lvds_dp_get_modes()
- Added a lock for mode setting
- Removed a few blank lines
- Changed the order at Makefile and Kconfig
drivers/gpu/drm/bridge/Kconfig | 11 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c | 395 +++++++++++++++++++++++++++++
3 files changed, 407 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index bd6acc8..1d02422 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -39,6 +39,17 @@ config DRM_DW_HDMI_AHB_AUDIO
Designware HDMI block. This is used in conjunction with
the i.MX6 HDMI driver.
+config DRM_GE_B850V3_LVDS_DP
+ tristate "GE B850v3 LVDS to DP++ display bridge"
+ depends on OF
+ select DRM_KMS_HELPER
+ select DRM_PANEL
+ ---help---
+ This is a driver for the display bridge of
+ GE B850v3 that convert dual channel LVDS
+ to DP++. This is used with the i.MX6 imx-ldb
+ driver.
+
config DRM_NXP_PTN3460
tristate "NXP PTN3460 DP/LVDS bridge"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 97ed1a5..b6b44a5 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
+obj-$(CONFIG_DRM_GE_B850V3_LVDS_DP) += ge_b850v3_lvds_dp.o
obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
obj-$(CONFIG_DRM_SII902X) += sii902x.o
diff --git a/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c b/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
new file mode 100644
index 0000000..85875d8
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
@@ -0,0 +1,395 @@
+/*
+ * Driver for GE B850v3 DP display bridge
+
+ * Copyright (c) 2016, Collabora Ltd.
+ * Copyright (c) 2016, General Electric Company
+
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+
+ * This program is distributed in the hope 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.
+
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+ * This driver creates a drm_bridge and a drm_connector for the LVDS to DP++
+ * display bridge of the GE B850v3. There are two physical bridges on the video
+ * signal pipeline: a STDP4028(LVDS to DP) and a STDP2690(DP to DP++). However
+ * the physical bridges are automatically configured by the input video signal,
+ * and the driver has no access to the video processing pipeline. The driver is
+ * only needed to read EDID from the STDP2690 and to handle HPD events from the
+ * STDP4028. The driver communicates with both bridges over i2c. The video
+ * signal pipeline is as follows:
+ *
+ * Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
+ *
+ */
+
+#include <linux/gpio.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_edid.h>
+#include <drm/drmP.h>
+
+/*
+ * 220Mhz is a limitation of the host, as the bridge is capable of up to
+ * 330Mhz. See section 9.2.1.2.4 of the i.MX 6Dual/6Quad Applications
+ * Processor Reference Manual for more information about the 220Mhz limit.
+ * The imx-ldb driver will warn about clocks over 170Mhz, but it seem to work
+ * fine.
+ */
+#define MAX_PIXEL_CLOCK 220000
+
+#define EDID_EXT_BLOCK_CNT 0x7E
+
+#define STDP4028_IRQ_OUT_CONF_REG 0x02
+#define STDP4028_DPTX_IRQ_EN_REG 0x3C
+#define STDP4028_DPTX_IRQ_STS_REG 0x3D
+#define STDP4028_DPTX_STS_REG 0x3E
+
+#define STDP4028_DPTX_DP_IRQ_EN 0x1000
+
+#define STDP4028_DPTX_HOTPLUG_IRQ_EN 0x0400
+#define STDP4028_DPTX_LINK_CH_IRQ_EN 0x2000
+#define STDP4028_DPTX_IRQ_CONFIG \
+ (STDP4028_DPTX_LINK_CH_IRQ_EN | STDP4028_DPTX_HOTPLUG_IRQ_EN)
+
+#define STDP4028_DPTX_HOTPLUG_STS 0x0200
+#define STDP4028_DPTX_LINK_STS 0x1000
+#define STDP4028_CON_STATE_CONNECTED \
+ (STDP4028_DPTX_HOTPLUG_STS | STDP4028_DPTX_LINK_STS)
+
+#define STDP4028_DPTX_HOTPLUG_CH_STS 0x0400
+#define STDP4028_DPTX_LINK_CH_STS 0x2000
+#define STDP4028_DPTX_IRQ_CLEAR \
+ (STDP4028_DPTX_LINK_CH_STS | STDP4028_DPTX_HOTPLUG_CH_STS)
+
+struct ge_b850v3_lvds_dp {
+ struct drm_connector connector;
+ struct drm_bridge bridge;
+ struct i2c_client *ge_b850v3_lvds_dp_i2c;
+ struct i2c_client *edid_i2c;
+ struct edid *edid;
+ struct mutex lock;
+};
+
+static inline struct ge_b850v3_lvds_dp *
+ bridge_to_ge_b850v3_lvds_dp(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct ge_b850v3_lvds_dp, bridge);
+}
+
+static inline struct ge_b850v3_lvds_dp *
+ connector_to_ge_b850v3_lvds_dp(struct drm_connector *connector)
+{
+ return container_of(connector, struct ge_b850v3_lvds_dp, connector);
+}
+
+u8 *stdp2690_get_edid(struct i2c_client *client)
+{
+ struct i2c_adapter *adapter = client->adapter;
+ unsigned char start = 0x00;
+ unsigned int total_size;
+ u8 *block = kmalloc(EDID_LENGTH, GFP_KERNEL);
+
+ struct i2c_msg msgs[] = {
+ {
+ .addr = client->addr,
+ .flags = 0,
+ .len = 1,
+ .buf = &start,
+ }, {
+ .addr = client->addr,
+ .flags = I2C_M_RD,
+ .len = EDID_LENGTH,
+ .buf = block,
+ }
+ };
+
+ if (!block)
+ return NULL;
+
+ if (i2c_transfer(adapter, msgs, 2) != 2) {
+ DRM_ERROR("Unable to read EDID.\n");
+ goto err;
+ }
+
+ if (!drm_edid_block_valid(block, 0, false, NULL)) {
+ DRM_ERROR("Invalid EDID block\n");
+ goto err;
+ }
+
+ total_size = (block[EDID_EXT_BLOCK_CNT] + 1) * EDID_LENGTH;
+ if (total_size > EDID_LENGTH) {
+ kfree(block);
+ block = kmalloc(total_size, GFP_KERNEL);
+ if (!block)
+ return NULL;
+
+ /* Yes, read the entire buffer, and do not skip the first
+ * EDID_LENGTH bytes.
+ */
+ start = 0x00;
+ msgs[1].len = total_size;
+ msgs[1].buf = block;
+
+ if (i2c_transfer(adapter, msgs, 2) != 2) {
+ DRM_ERROR("Unable to read EDID extension blocks.\n");
+ goto err;
+ }
+ }
+
+ return block;
+
+err:
+ kfree(block);
+ return NULL;
+}
+
+static int ge_b850v3_lvds_dp_get_modes(struct drm_connector *connector)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge;
+ struct i2c_client *client;
+ int num_modes = 0;
+
+ ptn_bridge = connector_to_ge_b850v3_lvds_dp(connector);
+ client = ptn_bridge->edid_i2c;
+
+ mutex_lock(&ptn_bridge->lock);
+
+ kfree(ptn_bridge->edid);
+ ptn_bridge->edid = (struct edid *) stdp2690_get_edid(client);
+
+ if (ptn_bridge->edid) {
+ drm_mode_connector_update_edid_property(connector,
+ ptn_bridge->edid);
+ num_modes = drm_add_edid_modes(connector, ptn_bridge->edid);
+ }
+
+ mutex_unlock(&ptn_bridge->lock);
+
+ return num_modes;
+}
+
+
+static enum drm_mode_status ge_b850v3_lvds_dp_mode_valid(
+ struct drm_connector *connector, struct drm_display_mode *mode)
+{
+ if (mode->clock > MAX_PIXEL_CLOCK) {
+ DRM_INFO("The pixel clock for the mode %s is too high, and not supported.",
+ mode->name);
+ return MODE_CLOCK_HIGH;
+ }
+
+ return MODE_OK;
+}
+
+static const struct
+drm_connector_helper_funcs ge_b850v3_lvds_dp_connector_helper_funcs = {
+ .get_modes = ge_b850v3_lvds_dp_get_modes,
+ .mode_valid = ge_b850v3_lvds_dp_mode_valid,
+};
+
+static enum drm_connector_status ge_b850v3_lvds_dp_detect(
+ struct drm_connector *connector, bool force)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge =
+ connector_to_ge_b850v3_lvds_dp(connector);
+ struct i2c_client *ge_b850v3_lvds_dp_i2c =
+ ptn_bridge->ge_b850v3_lvds_dp_i2c;
+ s32 link_state;
+
+ link_state = i2c_smbus_read_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_DPTX_STS_REG);
+
+ if (link_state == STDP4028_CON_STATE_CONNECTED)
+ return connector_status_connected;
+
+ if (link_state == 0)
+ return connector_status_disconnected;
+
+ return connector_status_unknown;
+}
+
+static const struct drm_connector_funcs ge_b850v3_lvds_dp_connector_funcs = {
+ .dpms = drm_atomic_helper_connector_dpms,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .detect = ge_b850v3_lvds_dp_detect,
+ .destroy = drm_connector_cleanup,
+ .reset = drm_atomic_helper_connector_reset,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static irqreturn_t ge_b850v3_lvds_dp_irq_handler(int irq, void *dev_id)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge = dev_id;
+ struct i2c_client *ge_b850v3_lvds_dp_i2c
+ = ptn_bridge->ge_b850v3_lvds_dp_i2c;
+
+ mutex_lock(&ptn_bridge->lock);
+
+ i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_DPTX_IRQ_STS_REG, STDP4028_DPTX_IRQ_CLEAR);
+
+ mutex_unlock(&ptn_bridge->lock);
+
+ if (ptn_bridge->connector.dev)
+ drm_kms_helper_hotplug_event(ptn_bridge->connector.dev);
+
+ return IRQ_HANDLED;
+}
+
+static int ge_b850v3_lvds_dp_attach(struct drm_bridge *bridge)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge
+ = bridge_to_ge_b850v3_lvds_dp(bridge);
+ struct drm_connector *connector = &ptn_bridge->connector;
+ struct i2c_client *ge_b850v3_lvds_dp_i2c
+ = ptn_bridge->ge_b850v3_lvds_dp_i2c;
+ int ret;
+
+ if (!bridge->encoder) {
+ DRM_ERROR("Parent encoder object not found");
+ return -ENODEV;
+ }
+
+ connector->polled = DRM_CONNECTOR_POLL_HPD;
+
+ drm_connector_helper_add(connector,
+ &ge_b850v3_lvds_dp_connector_helper_funcs);
+
+ ret = drm_connector_init(bridge->dev, connector,
+ &ge_b850v3_lvds_dp_connector_funcs,
+ DRM_MODE_CONNECTOR_DisplayPort);
+ if (ret) {
+ DRM_ERROR("Failed to initialize connector with drm\n");
+ return ret;
+ }
+
+ ret = drm_mode_connector_attach_encoder(connector, bridge->encoder);
+ if (ret)
+ return ret;
+
+ /* Configures the bridge to re-enable interrupts after each ack. */
+ i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_IRQ_OUT_CONF_REG, STDP4028_DPTX_DP_IRQ_EN);
+
+ /* Enable interrupts */
+ i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_DPTX_IRQ_EN_REG, STDP4028_DPTX_IRQ_CONFIG);
+
+ return 0;
+}
+
+static const struct drm_bridge_funcs ge_b850v3_lvds_dp_funcs = {
+ .attach = ge_b850v3_lvds_dp_attach,
+};
+
+static int ge_b850v3_lvds_dp_probe(struct i2c_client *ge_b850v3_lvds_dp_i2c,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &ge_b850v3_lvds_dp_i2c->dev;
+ struct ge_b850v3_lvds_dp *ptn_bridge;
+ int ret;
+ u32 edid_i2c_reg;
+
+ ptn_bridge = devm_kzalloc(dev, sizeof(*ptn_bridge), GFP_KERNEL);
+ if (!ptn_bridge)
+ return -ENOMEM;
+
+ mutex_init(&ptn_bridge->lock);
+
+ ptn_bridge->ge_b850v3_lvds_dp_i2c = ge_b850v3_lvds_dp_i2c;
+ ptn_bridge->bridge.driver_private = ptn_bridge;
+ i2c_set_clientdata(ge_b850v3_lvds_dp_i2c, ptn_bridge);
+
+ ret = of_property_read_u32(dev->of_node, "edid-reg", &edid_i2c_reg);
+ if (ret) {
+ dev_err(dev, "edid-reg not specified, aborting...\n");
+ return -ENODEV;
+ }
+
+ ptn_bridge->edid_i2c = devm_kzalloc(dev,
+ sizeof(struct i2c_client), GFP_KERNEL);
+
+ if (!ptn_bridge->edid_i2c)
+ return -ENOMEM;
+
+ memcpy(ptn_bridge->edid_i2c, ge_b850v3_lvds_dp_i2c,
+ sizeof(struct i2c_client));
+
+ ptn_bridge->edid_i2c->addr = (unsigned short) edid_i2c_reg;
+
+ ptn_bridge->bridge.funcs = &ge_b850v3_lvds_dp_funcs;
+ ptn_bridge->bridge.of_node = dev->of_node;
+ ret = drm_bridge_add(&ptn_bridge->bridge);
+ if (ret) {
+ DRM_ERROR("Failed to add bridge\n");
+ return ret;
+ }
+
+ /* Clear pending interrupts since power up. */
+ i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_DPTX_IRQ_STS_REG, STDP4028_DPTX_IRQ_CLEAR);
+
+ if (ge_b850v3_lvds_dp_i2c->irq) {
+ ret = devm_request_threaded_irq(&ge_b850v3_lvds_dp_i2c->dev,
+ ge_b850v3_lvds_dp_i2c->irq, NULL,
+ ge_b850v3_lvds_dp_irq_handler,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ "ge-b850v3-lvds-dp", ptn_bridge);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ge_b850v3_lvds_dp_remove(struct i2c_client *ge_b850v3_lvds_dp_i2c)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge =
+ i2c_get_clientdata(ge_b850v3_lvds_dp_i2c);
+
+ drm_bridge_remove(&ptn_bridge->bridge);
+
+ kfree(ptn_bridge->edid);
+
+ return 0;
+}
+
+static const struct i2c_device_id ge_b850v3_lvds_dp_i2c_table[] = {
+ {"b850v3-lvds-dp", 0},
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, ge_b850v3_lvds_dp_i2c_table);
+
+static const struct of_device_id ge_b850v3_lvds_dp_match[] = {
+ { .compatible = "ge,b850v3-lvds-dp" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ge_b850v3_lvds_dp_match);
+
+static struct i2c_driver ge_b850v3_lvds_dp_driver = {
+ .id_table = ge_b850v3_lvds_dp_i2c_table,
+ .probe = ge_b850v3_lvds_dp_probe,
+ .remove = ge_b850v3_lvds_dp_remove,
+ .driver = {
+ .name = "b850v3-lvds-dp",
+ .of_match_table = ge_b850v3_lvds_dp_match,
+ },
+};
+module_i2c_driver(ge_b850v3_lvds_dp_driver);
+
+MODULE_AUTHOR("Peter Senna Tschudin <peter.senna@collabora.com>");
+MODULE_AUTHOR("Martyn Welch <martyn.welch@collabora.co.uk>");
+MODULE_DESCRIPTION("GE LVDS to DP++ display bridge)");
+MODULE_LICENSE("GPL v2");
--
2.5.5
^ permalink raw reply related
* [PATCH V6 2/4] MAINTAINERS: Add entry for GE B850v3 LVDS/DP++ Bridge
From: Peter Senna Tschudin @ 2016-10-27 13:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1477572814.git.peter.senna@collabora.com>
Update the MAINTAINERS file for the GE B850v3 LVDS/DP++ Bridge.
Cc: Martyn Welch <martyn.welch@collabora.co.uk>
Cc: Martin Donnelly <martin.donnelly@ge.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
CC: David Airlie <airlied@linux.ie>
CC: Thierry Reding <treding@nvidia.com>
CC: Thierry Reding <thierry.reding@gmail.com>
CC: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 6834df3..23100f4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5249,6 +5249,14 @@ W: https://linuxtv.org
S: Maintained
F: drivers/media/radio/radio-gemtek*
+GENERAL ELECTRIC B850V3 LVDS/DP++ BRIDGE
+M: Peter Senna Tschudin <peter.senna@collabora.com>
+M: Martin Donnelly <martin.donnelly@ge.com>
+M: Martyn Welch <martyn.welch@collabora.co.uk>
+S: Maintained
+F: drivers/gpu/drm/bridge/ge_b850v3_dp2.c
+F: Documentation/devicetree/bindings/ge/b850v3_dp2_bridge.txt
+
GENERIC GPIO I2C DRIVER
M: Haavard Skinnemoen <hskinnemoen@gmail.com>
S: Supported
--
2.5.5
^ permalink raw reply related
* [PATCH V6 1/4] Documentation/devicetree/bindings: b850v3_lvds_dp
From: Peter Senna Tschudin @ 2016-10-27 13:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1477572814.git.peter.senna@collabora.com>
Devicetree bindings documentation for the GE B850v3 LVDS/DP++
display bridge.
Cc: Martyn Welch <martyn.welch@collabora.co.uk>
Cc: Martin Donnelly <martin.donnelly@ge.com>
Cc: Javier Martinez Canillas <javier@dowhile0.org>
Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Archit Taneja <architt@codeaurora.org>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
.../devicetree/bindings/ge/b850v3-lvds-dp.txt | 37 ++++++++++++++++++++++
1 file changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt
diff --git a/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt b/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt
new file mode 100644
index 0000000..f05c3e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt
@@ -0,0 +1,37 @@
+Driver for GE B850v3 LVDS/DP++ display bridge
+
+Required properties:
+ - compatible : should be "ge,b850v3-lvds-dp".
+ - reg : should contain the address used to ack the interrupts.
+ - interrupt-parent : phandle of the interrupt controller that services
+ interrupts to the device
+ - interrupts : one interrupt should be described here, as in
+ <0 IRQ_TYPE_LEVEL_HIGH>.
+ - edid-reg : should contain the address used to read edid information
+ - port : should describe the video signal connection between the host
+ and the bridge.
+
+Example:
+
+&mux2_i2c2 {
+ status = "okay";
+ clock-frequency = <100000>;
+
+ b850v3-lvds-dp-bridge at 73 {
+ compatible = "ge,b850v3-lvds-dp";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ reg = <0x73>;
+ interrupt-parent = <&gpio2>;
+ interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+
+ edid-reg = <0x72>;
+
+ port {
+ b850v3_dp_bridge_in: endpoint {
+ remote-endpoint = <&lvds0_out>;
+ };
+ };
+ };
+};
--
2.5.5
^ permalink raw reply related
* [PATCH V6 0/4] Add driver for GE B850v3 LVDS/DP++ Bridge
From: Peter Senna Tschudin @ 2016-10-27 13:01 UTC (permalink / raw)
To: linux-arm-kernel
The series adds a driver that creates a drm_bridge and a drm_connector for the
LVDS to DP++ display bridge of the GE B850v3.
There are two physical bridges on the video signal pipeline: a STDP4028(LVDS to
DP) and a STDP2690(DP to DP++). The hardware and firmware made it complicated
for this binding to comprise two device tree nodes, as the design goal is to
configure both bridges based on the LVDS signal, which leave the driver
powerless to control the video processing pipeline. The two bridges behaves as
a single bridge, and the driver is only needed for telling the host about EDID /
HPD, and for giving the host powers to ack interrupts. The video signal
pipeline is as follows:
Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
This series depends on commit dc80d7038883 ("drm/imx-ldb: Add support to
drm-bridge") which is already on linux-next.
The patches from the series:
[1/4] Devicetree documentation for the GE B850v3 LVDS/DP++ Bridge
[2/4] Update the MAINTAINERS file
[3/4] Add the driver, make changes to Kconfig and Makefile
[4/4] Make the changes to the B850v3 dts file to enable the GE B850v3
LVDS/DP++ Bridge.
Tested on next20161027.
Changes from V5:
- Change to MAINTAINERS in a separate patch
- Reworked interrupt handler initialization
- Removed useless calls to: drm_connector_register(),
drm_helper_hpd_irq_event(), and drm_bridge_enable()
Changes from V4:
- Renamed the i2c_driver.name from "ge,b850v3-lvds-dp" to "b850v3-lvds-dp" to
remove the comma from the driver name
Changes from V3:
- Removed the patch that was configuring the mapping between IPUs and external
displays on the dts file
Peter Senna Tschudin (4):
Documentation/devicetree/bindings: b850v3_lvds_dp
MAINTAINERS: Add entry for GE B850v3 LVDS/DP++ Bridge
drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge
dts/imx6q-b850v3: Use GE B850v3 LVDS/DP++ Bridge
.../devicetree/bindings/ge/b850v3-lvds-dp.txt | 37 ++
MAINTAINERS | 8 +
arch/arm/boot/dts/imx6q-b850v3.dts | 30 ++
drivers/gpu/drm/bridge/Kconfig | 11 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c | 395 +++++++++++++++++++++
6 files changed, 482 insertions(+)
create mode 100644 Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt
create mode 100644 drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
--
2.5.5
^ permalink raw reply
* Add Allwinner Q8 tablets hardware manager
From: Pierre-Hugues Husson @ 2016-10-27 12:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <99dde1e0-26f0-cc7b-36bf-1181c47296f9@redhat.com>
2016-10-27 11:14 GMT+02:00 Hans de Goede <hdegoede@redhat.com>:
> In my experience with these cheap boards, there is a mix of auto-probing +
> device / revision specific os-image modifications. I keep coming back to
> the touchscreen controller firmware (but also the orientation), for the
> gsl1680 controller I need at least 2 different firmware files (per gsl1680
> revision) to make all q8 tablets I have working. This is simply not solved
> by the vendor android code, they just shove the right firmware into the
> os-image. Likewise for the touchscreen orientation (x-mirored, y-mirored,
> etc) too is just a hard-coded setting in the os-image.
Reading your patch, it looks like to handle the two different firmware
files, you're simply adding a command-line switch, there is no
detection involved.
Am I understanding correctly?
If this is the case, two things:
1. I'm not too sure having the user choose this via cmdline is the
right way. I think I'd rather have it set by userspace. (though that's
not a strong opinion).
Or if cmdline is being changed... how about having DTS (or just an
overlay on top of it) being changed instead?
2. This could still be declared by DTS. For instance, assuming your
i2c-probe-stop-at-first-match:
&i2c0 {
touchscreen1: gsl1680 at 40 {
reg = <0x40>;
compatible = "silead,gsl1680";
enable-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
touchscreen-size = <1024 600>;
touchscreen-fw = "gsl1680-a082-q8-700.fw";
filter-names = "touchscreen_variant";
filter-0 = "none", "gsl1680-a082-q8-700";
id = <0xa0820000>;
status = "disabled";
};
touchscreen2: gsl1680 at 40 {
reg = <0x40>;
compatible = "silead,gsl1680";
enable-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
touchscreen-size = <480 800>;
touchscreen-fw = "gsl1680-a082-q8-a70.fw";
filter-names = "touchscreen_variant";
filter-0 = "gsl1680-a082-q8-a70";
id = <0xa0820000>;
status = "disabled";
};
touchscreen2: gsl1680 at 40 {
reg = <0x40>;
compatible = "silead,gsl1680";
enable-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
touchscreen-size = <960 640>;
touchscreen-fw = "gsl1680-b482-q8-d702.fw";
filter-names = "touchscreen_variant";
filter-0 = "gsl1680-b482-q8-d702";
id = <0xb4820000>;
status = "disabled";
};
i2c-probe-stop-at-first-match = <&touchscreen1>,
<&touchscreen2>, <&touchscreen3>;
}
With "none" value being the value when the "touchscreen_variant"
option is not defined in cmdline.
Please note that I'm not too sure whether SILEAD_REG_ID represents an
OTP which can be changed by OEM, or if it's more of a hardware
revision. Depending on this, this would either fit into a id =
<0xa0820000> DTS line, or a compatible = "silead,gsl1680_a082",
"silead,gsl1680"; DTS line.
> Sofar I've only seen this with one type of touchscreen so an easy cop-out
> would be to add an "optional-vddio-supply" to the the bindings for the
> specific touchscreen use and put all the necessary logic in the driver.
>
> This does require propagating the learned need for the regulator
> from the drivers detect() callback to probe() or alternatively I'm
> thinking we should just use probe() instead of detect()to begin with,
> that will save a lot of duplication with things
> like code for enable gpio-s and regulators.
>
> So assuming we go for the cop-out option for 3. (I'm ok with that),
> this would be a pretty clean solution adding just the 2 new:
> i2c-probe-stop-at-first-match and i2c-probe-all properties to
> the i2c-bus bindings. One problem here is that we may want to have
> multiple i2c-probe-stop-at-first-match phandle lists on a single bus
> (e.g. try 3 touchscreens + 6 accelerometers on the same bus, stop at
> first touchscreen / first accelerometer), anyone have any ideas for
> that?
How about something like:
&i2c1 {
touchscreen1....
touchscreen2....
touchscreen3....
accelerometer1....
accelerometer2....
accelerometer3....
accelerometer4....
select-one {
compatible = "i2c-select;
group-names = "touchscreen", "accelerometer";
group-0 = <&touchscreen1>, <&touchscreen2>, <&touchscreen3>;
group-1 = <&accelerometer1>, <&accelerometer2>,
<&accelerometer3>, <&accelerometer4>;
};
};
>> When it comes to detection, I've witnessed various things.
>> It can be kernel-side or bootloader-side "global setting" reading (like an
>> ADC/resistor value, or an OTP), it can be bootloader doing the
>> "brute-force", or it can be the kernel doing all the probes.
>>
>> For instance, as of today, on a Spreadtrum ODM tree, the bootloader will
>> detect the screen by testing all knowns screens, the screen-drivers declare
>> a get_id function, and the bootloader probes until the get_id matches the id
>> declared by the screen driver.
>> And then the bootloader tells the kernel, via cmdline, which screen is
>> actually there (but auto-detection is also coded in kernel).
>> Finally all possible sensors/touchscreen/camera are declared in DTS, and
>> probe will filter-out N/C ones in the kernel.
>>
>> Now the big difference between my experience and what Hans is trying to
>> do, is that I've always worked with devices with "safely" queriable IDs,
>> either on i2c or dsi. I've never encountered SPI. This makes probing
>> inherently more dangerous, but I believe the question roughly remains the
>> same.
>
>
> I'm dealing with i2c too, Mark mistakenly used SPI in his reply,
> which I think is what got you thinking I've SPI.
Right, so let's concentrate on reasonable bus-es first then. (I can
think of I2C and DSI)
> See above, I think that we can make this work by delegating the actual
> detection to the driver (so each compatible can have a different detect
> method / code).
> So with this we can remove a big part of drivers/misc/q8-hardwaremgr.c, but
> not all
> of it. We still need board specific code somewhere to deal with things like
> picking
> the right touchscreen firmware and touchscreen orientation. This is all
> somewhat
> gsl1680 specific.
> I actually have the same problem on x86 where the ACPI description of the
> device
> basically says: "There is a gsl1680 at this bus at this address" and does
> not say
> anything about firmware / orientation (again this is simply hardcoded
> in the os-image these devices ship with).
>
> For x86 my plan is to have an array of configs in the driver and select the
> right
> one based on DMI strings, which is in essence putting board specific info in
> the
> driver.
>
> I can imagine mirroring this for ARM, and have an array of configs in the
> driver
> there too (for cases where cannot simply hardcode everything in dt only) and
> have
> some board specific code (activated by of_machine_is_compatible()) to select
> the
> right config.
I do believe this can all be done in DTS, and at the moment, what
you're describing seem to happen often enough to be worth writing
generic code for.
But then, I can't really tell which makes the most sense between
source-based and devicetree-based.
I prefer doing it in device-tree, since it means that any OEM can have
his device supported by only providing DTB, and won't need to provide
kernel patches.
> 2) miscellaneous extra config on top of figuring out which ICs are
> connected,
> basically the kind of stuff many vendors simply hard-code in their device
> specific os-image. This one is much more difficult to deal with and I think
> we need to figure this out on a case by case basis. This will require board
> specific code (just like the kernel has tons of DMI string activated board
> specific code on x86) and what is the best code for this place to live will
> be a case by case thing too.
With things like mount-matrix devicetree property, the goal is to have
such informations in the DTS.
I'm not seeing every cases should be handled in DTS, but ATM I see no
good reason.
^ permalink raw reply
* [PATCH] ARM: DT: stm32: move dma translation to board files
From: Bruno Herrera @ 2016-10-27 12:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <65096fbe-3f06-c8db-fbe8-29f0be28cb61@st.com>
Hi Alex,
On Thu, Oct 27, 2016 at 10:21 AM, Alexandre Torgue
<alexandre.torgue@st.com> wrote:
> Hi Bruno,
>
>
> On 10/27/2016 12:43 PM, Bruno Herrera wrote:
>>
>> Hi Alex,
>>
>> On Wed, Oct 26, 2016 at 7:09 AM, Alexandre Torgue
>> <alexandre.torgue@st.com> wrote:
>>>
>>> Hi Bruno,
>>>
>>> On 10/25/2016 11:06 PM, Bruno Herrera wrote:
>>>>
>>>>
>>>> Hi Alexandre,
>>>>
>>>>>
>>>>> stm32f469-disco and stm32f429-eval boards use SDRAM start address
>>>>> remapping
>>>>> (to @0) to boost performances. A DMA translation through "dma-ranges"
>>>>> property was needed for other masters than the M4 CPU.
>>>>> stm32f429-disco doesn't use remapping so doesn't need this DMA
>>>>> translation.
>>>>> This patches moves this DMA translation definition from stm32f429 soc
>>>>> file
>>>>> to board files.
>>>>>
>>>>> Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/stm32429i-eval.dts
>>>>> b/arch/arm/boot/dts/stm32429i-eval.dts
>>>>> index 13c7cd2..a763c15 100644
>>>>> --- a/arch/arm/boot/dts/stm32429i-eval.dts
>>>>> +++ b/arch/arm/boot/dts/stm32429i-eval.dts
>>>>> @@ -82,6 +82,10 @@
>>>>> };
>>>>> };
>>>>>
>>>>> + soc {
>>>>> + dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>>> + };
>>>>> +
>>>>> usbotg_hs_phy: usbphy {
>>>>> #phy-cells = <0>;
>>>>> compatible = "usb-nop-xceiv";
>>>>
>>>>
>>>>
>>>> Shouldn't also the peripheral dma-ranges property move to board specific
>>>> too?
>>>> I had this patch for while but I didn't had the time to submit:
>>>
>>>
>>>
>>> Well spot I forgot it. Actually, discussing with Arnd ysterday on IIRC,
>>> empty dma-ranges is not needed. Can you test on your side by removing
>>> dma-ranges in usb node please ?
>>
>> Unfortunately will take a time for me to set up this environment on
>> the STM32F4-EVAL board.
>> And on the discovery boards we dont have this scenario. That was the
>> main reason I did not submit the patch right away.
>> My conclusion and I might be wrong but is based on the my tests with
>> SDIO device at STM32F469I-DISCO board.
>>
>> I started this issue as discussion at ST Forum but Maxime gave me the
>> hint.
>>
>>
>> https://my.st.com/public/STe2ecommunities/mcu/Lists/cortex_mx_stm32/Flat.aspx?RootFolder=https%3a%2f%2fmy%2est%2ecom%2fpublic%2fSTe2ecommunities%2fmcu%2fLists%2fcortex_mx_stm32%2fDMA2%20and%20SYSCFG_MEMRMP%20relationship&FolderCTID=0x01200200770978C69A1141439FE559EB459D7580009C4E14902C3CDE46A77F0FFD06506F5B¤tviews=44
>>
>>> I will push a v2 by removing empty dma-ranges if tests are ok in your
>>> side.
>>
>>
>> From my understating/conclusion is: when empty property(dma-ranges) is
>> the device node, the mapping will be taken in consideration when using
>> DMA otherwise the mapping is ignored.
>> And in the SDIO case it is needed for DEV->MEM(SDRAM) and
>> MEM(SDRAM)->DEV. If it is not the case for the devices in question so
>> I suppose it can work without the property.
>
>
> For sure translation has to be done but I'm not sure that an empty
> "dma-ranges" is needed in device node to activate it. For Ethernet empty
> "dma-ranges" is not needed. I will try with usb.
In the case of SDIO it is needed. As example this is my working SDIO node:
sdio: sdio at 40012c00 {
compatible = "arm,pl18x", "arm,primecell";
arm,primecell-periphid = <0x00480181>;
reg = <0x40012c00 0x400>;
dmas = <&dma2 6 4 0x10400 0x3>, /* Logical - DevToMem */
<&dma2 3 4 0x10400 0x3>; /* Logical - MemToDev */
dma-names = "rx", "tx";
clocks = <&rcc 0 171>;
clock-names = "apb_pclk";
interrupts = <49>;
status = "disabled";
};
&sdio {
status = "okay";
vmmc-supply = <&wlan_en>;
bus-width = <4>;
max-frequency = <24000000>;
pinctrl-names = "default";
pinctrl-0 = <&sdio_pins>;
ti,non-removable;
ti,needs-special-hs-handling;
dma-ranges;
cap-power-off-card;
keep-power-in-suspend;
#address-cells = <1>;
#size-cells = <0>;
wlcore: wlcore at 0 {
compatible = "ti,wl1835";
reg = <2>;
interrupt-parent = <&gpioa>;
interrupts = <8 IRQ_TYPE_EDGE_RISING>;
};
};
>
> alex
>
>
>>
>>>
>>> Thanks in advance
>>> Alex
>>>
>>>
>>>>
>>>> Author: Bruno Herrera <bruherrera@gmail.com>
>>>> Date: Sun Oct 16 14:50:00 2016 -0200
>>>>
>>>> ARM: DT: STM32: Use dma-ranges property per board not at dtsi file
>>>>
>>>> diff --git a/arch/arm/boot/dts/stm32429i-eval.dts
>>>> b/arch/arm/boot/dts/stm32429i-eval.dts
>>>> index 6bfc595..2a22a82 100644
>>>> --- a/arch/arm/boot/dts/stm32429i-eval.dts
>>>> +++ b/arch/arm/boot/dts/stm32429i-eval.dts
>>>> @@ -52,6 +52,10 @@
>>>> model = "STMicroelectronics STM32429i-EVAL board";
>>>> compatible = "st,stm32429i-eval", "st,stm32f429";
>>>>
>>>> + soc {
>>>> + dma-ranges = <0xC0000000 0x0 0x10000000>;
>>>> + };
>>>> +
>>>> chosen {
>>>> bootargs = "root=/dev/ram rdinit=/linuxrc";
>>>> stdout-path = "serial0:115200n8";
>>>> @@ -96,6 +100,7 @@
>>>>
>>>> ðernet0 {
>>>> status = "okay";
>>>> + dma-ranges;
>>>> pinctrl-0 = <ðernet0_mii>;
>>>> pinctrl-names = "default";
>>>> phy-mode = "mii-id";
>>>> @@ -116,6 +121,7 @@
>>>> };
>>>>
>>>> &usbotg_hs {
>>>> + dma-ranges;
>>>> dr_mode = "host";
>>>> phys = <&usbotg_hs_phy>;
>>>> phy-names = "usb2-phy";
>>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>>> index 7d624a2..697a133 100644
>>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>>> @@ -59,7 +59,6 @@
>>>> };
>>>>
>>>> soc {
>>>> - dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>>
>>>> timer2: timer at 40000000 {
>>>> compatible = "st,stm32-timer";
>>>> @@ -472,13 +471,11 @@
>>>> st,syscon = <&syscfg 0x4>;
>>>> snps,pbl = <8>;
>>>> snps,mixed-burst;
>>>> - dma-ranges;
>>>> status = "disabled";
>>>> };
>>>>
>>>> usbotg_hs: usb at 40040000 {
>>>> compatible = "snps,dwc2";
>>>> - dma-ranges;
>>>> reg = <0x40040000 0x40000>;
>>>> interrupts = <77>;
>>>> clocks = <&rcc 0 29>;
>>>>
>>>>
>>>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>>>> index 0596d60..3a1cfdd 100644
>>>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>>>> @@ -59,8 +59,6 @@
>>>>> };
>>>>>
>>>>> soc {
>>>>> - dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>>> -
>>>>> timer2: timer at 40000000 {
>>>>> compatible = "st,stm32-timer";
>>>>> reg = <0x40000000 0x400>;
>>>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>>>>> b/arch/arm/boot/dts/stm32f469-disco.dts
>>>>> index 9e73656..c2213c0 100644
>>>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>>>> @@ -64,6 +64,10 @@
>>>>> aliases {
>>>>> serial0 = &usart3;
>>>>> };
>>>>> +
>>>>> + soc {
>>>>> + dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>>> + };
>>>>> };
>>>>>
>>>>> &clk_hse {
>>>>> --
>>>>
>>>>
>>>>
>>>>
>>>> Br.,
>>>> Bruno
>>>>
>>>
>
^ permalink raw reply
* [PATCH v3] drivers: psci: PSCI checker module
From: Kevin Brodsky @ 2016-10-27 12:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161027091306.GA16603@red-moon>
On 27/10/16 10:13, Lorenzo Pieralisi wrote:
> On Wed, Oct 26, 2016 at 11:11:48AM -0700, Paul E. McKenney wrote:
>> On Wed, Oct 26, 2016 at 06:35:34PM +0100, Lorenzo Pieralisi wrote:
>>> On Wed, Oct 26, 2016 at 10:22:52AM -0700, Paul E. McKenney wrote:
>>>> On Wed, Oct 26, 2016 at 06:10:06PM +0100, Lorenzo Pieralisi wrote:
>> [ . . . ]
>>
>>>>> Thanks a lot for your feedback, thoughts appreciated.
>>>> Let me ask the question more directly.
>>>>
>>>> Why on earth are we trying to run these tests concurrently?
>>> We must prevent that, no question about that, that's why I started
>>> this discussion. It is not fine to enable this checker and the
>>> RCU/LOCK torture hotplug tests at the same time.
>>>
>>>> After all, if we just run one at a time in isolation, there is no
>>>> problem.
>>> Fine by me, it was to understand if the current assumptions we made
>>> are correct and they are definitely not. If we enable the PSCI checker
>>> we must disable the torture rcu/lock hotplug tests either statically or
>>> dynamically.
>> What rcutorture, locktorture, and rcuperf do is to invoke
>> torture_init_begin(), which returns false if one of these tests
>> is already running.
>>
>> Perhaps we should extract this torture-test-exclusion and require
>> than conflicting torture tests invoke it?
> Yes if it can be extracted as a check (but it should also prevent the
> torture tests from running and vice versa), either that or Kconfig
> dependency (which we could do as a first step, waiting to add the
> required interface to the torture test code ?).
>
> Thanks !
> Lorenzo
That sounds like a reasonable idea, but then that would mean that the PSCI checker
would have to wait until the torture test is finished if it is already running (and
the other way around).
I wasn't aware that torture tests were hotplugging CPUs. I think that the most
sensible thing to do right now is to make CONFIG_PSCI_CHECKER depend on
!CONFIG_TORTURE_TEST (or maybe specifically !CONFIG_RCU_TORTURE_TEST &&
!CONFIG_LOCK_TORTURE_TEST). We can try to make them work together afterwards, but for
the sake of getting this patch merged in a reasonable amount of time, I think we
should just exclude the conflicting tests at the build level in this patch. I'll also
update the comment accordingly.
Thanks,
Kevin
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply
* [PATCH] arm/arm64: KVM: Perform local TLB invalidation when multiplexing vcpus on a single CPU
From: Christoffer Dall @ 2016-10-27 12:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161027104925.GC27135@leverpostej>
On Thu, Oct 27, 2016 at 11:51:26AM +0100, Mark Rutland wrote:
> Hi Christoffer,
>
> On Thu, Oct 27, 2016 at 12:04:28PM +0200, Christoffer Dall wrote:
> > On Thu, Oct 27, 2016 at 10:49:00AM +0100, Marc Zyngier wrote:
> > > The guest wouldn't have to do any invalidation at all on real HW,
> > > because the TLBs are strictly private to a physical CPU (only the
> > > invalidation can be broadcast to the Inner Shareable domain). But when
> > > we multiplex two vcpus on the same physical CPU, we break the private
> > > semantics, and a vcpu could hit in the TLB entries populated by the
> > > another one.
> >
> > Such a guest would be using a mapping of the same VA with the same ASID
> > on two separate CPUs, each pointing to a separate PA. If it ever were
> > to, say, migrate a task, it would have to do invalidations then. Right?
>
> An OS (not Linux) could use a different ASID space per-cpu.
>
> e.g. with two single-threaded tasks A and B, you could have ASIDS:
>
> cpu0 cpu1
> A 0 1
> B 1 0
>
> ... and this would not be a problem, so long as when mappings changed
> maintenance were performed appropriately (e.g. perhaps it uses IPIs to
> trigger the relevant local TLB invlidation, rather than using broadcast
> ops).
>
> > Does Linux or other guests actually do this?
>
> Linux currently doesn't use ASIDs that way, but does use global mappings
> in a potentially-confliciting way in the cold-return paths (hotplug-on
> and return from idle). With two vCPUs, you could have a sequence like:
>
> cpu0 cpu1
> Task with ASID x started
> hotplug on
> install global TTBR0 mapping
> global entry allocated into TLB
> Task hits cpu1's global entry
>
> ... which cannot happen bare-metal, and there's no point at which the
> guest can perform suitable maintenance.
>
> > Another fix would be to allocate a VMID per VCPU I suppose, just to
> > introduce a terrible TLB hit ratio :)
>
> That would break broadcast invalidation within the guest, no?
>
> ... unless you also trapped all TLB maintenance, and did the IPI-based
> broadcast in SW.
>
Thanks for explanations, I'm getting the full picture now.
-Christoffer
^ permalink raw reply
* [PATCH] arm/arm64: KVM: Perform local TLB invalidation when multiplexing vcpus on a single CPU
From: Christoffer Dall @ 2016-10-27 12:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f00b6e69-7b45-5238-4d3a-af8d77e85670@arm.com>
On Thu, Oct 27, 2016 at 11:40:00AM +0100, Marc Zyngier wrote:
> On 27/10/16 11:04, Christoffer Dall wrote:
> > On Thu, Oct 27, 2016 at 10:49:00AM +0100, Marc Zyngier wrote:
> >> Hi Christoffer,
> >>
> >> On 27/10/16 10:19, Christoffer Dall wrote:
> >>> On Mon, Oct 24, 2016 at 04:31:28PM +0100, Marc Zyngier wrote:
> >>>> Architecturally, TLBs are private to the (physical) CPU they're
> >>>> associated with. But when multiple vcpus from the same VM are
> >>>> being multiplexed on the same CPU, the TLBs are not private
> >>>> to the vcpus (and are actually shared across the VMID).
> >>>>
> >>>> Let's consider the following scenario:
> >>>>
> >>>> - vcpu-0 maps PA to VA
> >>>> - vcpu-1 maps PA' to VA
> >>>>
> >>>> If run on the same physical CPU, vcpu-1 can hit TLB entries generated
> >>>> by vcpu-0 accesses, and access the wrong physical page.
> >>>>
> >>>> The solution to this is to keep a per-VM map of which vcpu ran last
> >>>> on each given physical CPU, and invalidate local TLBs when switching
> >>>> to a different vcpu from the same VM.
> >>>
> >>> Just making sure I understand this: The reason you cannot rely on the
> >>> guest doing the necessary distinction with ASIDs or invalidating the TLB
> >>> is that a guest (which assumes it's running on hardware) can validly
> >>> defer any neccessary invalidation until it starts running on other
> >>> physical CPUs, but we do this transparently in KVM?
> >>
> >> The guest wouldn't have to do any invalidation at all on real HW,
> >> because the TLBs are strictly private to a physical CPU (only the
> >> invalidation can be broadcast to the Inner Shareable domain). But when
> >> we multiplex two vcpus on the same physical CPU, we break the private
> >> semantics, and a vcpu could hit in the TLB entries populated by the
> >> another one.
> >
> > Such a guest would be using a mapping of the same VA with the same ASID
> > on two separate CPUs, each pointing to a separate PA. If it ever were
> > to, say, migrate a task, it would have to do invalidations then. Right?
>
> This doesn't have to be ASID tagged. Actually, it is more likely to
> affect global mappings. Imagine for example that the kernel (which uses
> global mappings for its own page tables) decides to create per-cpu
> variable using this trick (all the CPUs have the same VA, but use
> different PAs). No invalidation at all, everything looks perfectly fine,
> until you start virtualizing it.
>
> > Does Linux or other guests actually do this?
>
> Linux may hit it with CPU hotplug, which uses global mappings (which a
> vcpu using an ASID tagged mapping could then hit if the VAs overlap).
>
Right, ok, it's more threatening than I first thought. Thanks for the
explanation.
> >
> > I would suspect Linux has to eventually invalidate those mappins if it
> > wants the scheduler to be allowed to freely move things around.
> >
> >>
> >> As we cannot segregate the TLB entries per vcpu (but only per VMID), the
> >> workaround is to nuke all the TLBs for this VMID (locally only - no
> >> broadcast) each time we find that two vcpus are sharing the same
> >> physical CPU.
> >>
> >> Is that clearer?
> >
> > Yes, the fix is clear, just want to make sure I understand that it's a
> > valid circumstance where this actually happens. But in either case, we
> > probably have to fix this to emulate the hardware correctly.
> >
> > Another fix would be to allocate a VMID per VCPU I suppose, just to
> > introduce a terrible TLB hit ratio :)
>
> But that would break TLB invalidations that are broadcast in the Inner
> Shareable domain. To do so, you'd have to trap every TBLI, and issue
> corresponding invalidations for all the vcpus. I'm not sure I want to
> see the performance number of that solution... ;-)
>
Ah, yeah, that's ridiculous. Forget what I said.
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH v2 3/4] arm64: arch_timer: Work around Erratum Hisilicon-161601
From: Marc Zyngier @ 2016-10-27 12:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <44cf2916-7c81-5cdf-d113-f99326eae2c6@huawei.com>
On 27/10/16 13:17, Ding Tianhong wrote:
>
>
> On 2016/10/27 18:58, Marc Zyngier wrote:
>> On 27/10/16 08:34, Ding Tianhong wrote:
>>> Erratum Hisilicon-161601 says that the ARM generic timer counter "has the
>>> potential to contain an erroneous value when the timer value changes".
>>> Accesses to TVAL (both read and write) are also affected due to the implicit counter
>>> read. Accesses to CVAL are not affected.
>>>
>>> The workaround is to reread the system count registers until the value of the second
>>> read is larger than the first one by less than 32, the system counter can be guaranteed
>>> not to return wrong value twice by back-to-back read and the error value is always larger
>>> than the correct one by 32. Writes to TVAL are replaced with an equivalent write to CVAL.
>>>
>>> The workaround is enabled if the hisilicon,erratum-161601 property is found in
>>> the timer node in the device tree. This can be overridden with the
>>> clocksource.arm_arch_timer.hisilicon-161601 boot parameter, which allows KVM
>>> users to enable the workaround until a mechanism is implemented to
>>> automatically communicate this information.
>>>
>>> Fix some description for fsl erratum a008585.
>>>
>>> v2: Significant rework based on feedback, including seperate the fsl erratum a008585
>>> to another patch, update the erratum name and remove unwanted code.
>>>
>>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>>> ---
>>> Documentation/arm64/silicon-errata.txt | 1 +
>>> Documentation/kernel-parameters.txt | 9 ++++
>>> arch/arm64/include/asm/arch_timer.h | 28 ++++++++++-
>>> drivers/clocksource/Kconfig | 14 +++++-
>>> drivers/clocksource/arm_arch_timer.c | 88 ++++++++++++++++++++++++++--------
>>> 5 files changed, 118 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
>>> index 405da11..70c5d5e 100644
>>> --- a/Documentation/arm64/silicon-errata.txt
>>> +++ b/Documentation/arm64/silicon-errata.txt
>>> @@ -63,3 +63,4 @@ stable kernels.
>>> | Cavium | ThunderX SMMUv2 | #27704 | N/A |
>>> | | | | |
>>> | Freescale/NXP | LS2080A/LS1043A | A-008585 | FSL_ERRATUM_A008585 |
>>> +| Hisilicon | Hip05/Hip06/Hip07 | #161601 | HISILICON_ERRATUM_161601|
>>
>> I've already commented on the alignment. Please read my initial review.
>>
>
> It sees misunderstood, fix it this time.
>
>>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>>> index 6fa1d8a..735b4b6 100644
>>> --- a/Documentation/kernel-parameters.txt
>>> +++ b/Documentation/kernel-parameters.txt
>>> @@ -707,6 +707,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>> erratum. If unspecified, the workaround is
>>> enabled based on the device tree.
>>>
>>> + clocksource.arm_arch_timer.hisilicon-161601=
>>> + [ARM64]
>>> + Format: <bool>
>>> + Enable/disable the workaround of Hisilicon
>>> + erratum 161601. This can be useful for KVM
>>> + guests, if the guest device tree doesn't show the
>>> + erratum. If unspecified, the workaround is
>>> + enabled based on the device tree.
>>> +
>>> clearcpuid=BITNUM [X86]
>>> Disable CPUID feature X for the kernel. See
>>> arch/x86/include/asm/cpufeatures.h for the valid bit
>>> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
>>> index 118719d8..49b3041 100644
>>> --- a/arch/arm64/include/asm/arch_timer.h
>>> +++ b/arch/arm64/include/asm/arch_timer.h
>>> @@ -29,7 +29,7 @@
>>>
>>> #include <clocksource/arm_arch_timer.h>
>>>
>>> -#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585)
>>> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>>> extern struct static_key_false arch_timer_read_ool_enabled;
>>> #define needs_timer_erratum_workaround() \
>>> static_branch_unlikely(&arch_timer_read_ool_enabled)
>>> @@ -65,11 +65,35 @@ extern struct arch_timer_erratum_workaround *erratum_workaround;
>>> _new; \
>>> })
>>>
>>> +
>>> +
>>> +/*
>>> + * The number of retries is an arbitrary value well beyond the highest number
>>> + * of iterations the loop has been observed to take.
>>> + * Verify whether the value of the second read is larger than the first by
>>> + * less than 32 is the only way to confirm the value is correct, the system
>>> + * counter can be guaranteed not to return wrong value twice by back-to-back read
>>> + * and the error value is always larger than the correct one by 32.
>>> + */
>>> +#define __hisi_161601_read_reg(reg) ({ \
>>> + u64 _old, _new; \
>>> + int _retries = 200; \
>>
>> Please document how this value was found (either in the code or in the
>> commit message).
>
> It really difficult to give the accurate standard, theoretically the error should not happened
> twice together, so maybe 2 is enough here, I just give a arbitrary value.
>
>>
>>> + \
>>> + do { \
>>> + _old = read_sysreg(reg); \
>>> + _new = read_sysreg(reg); \
>>> + _retries--; \
>>> + } while (unlikely((_new - _old) >> 5) && _retries); \
>>> + \
>>> + WARN_ON_ONCE(!_retries); \
>>> + _new; \
>>> +})
>>
>> Same remark as in the previous patch.
>>
>
> I think the sentence *Verify whether the value of the second read is larger than the first by
> less than 32 is the only way to confirm the value is correct* could explain why should *(_new - _old) >> 5*.
> it is the same as (_new - _old)/32, also mean the _new should never bigger than _old more than 32.
This is not about the explanation of the erratum, but about the location
of the #define, which can be made private to the .c file instead of
being globally visible.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH] ARM: DT: stm32: move dma translation to board files
From: Alexandre Torgue @ 2016-10-27 12:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAF3+TqdzESUNbkmAVv42pNv13wuyuPgGgksK3pJbORYw2a8ZFQ@mail.gmail.com>
Hi Bruno,
On 10/27/2016 12:43 PM, Bruno Herrera wrote:
> Hi Alex,
>
> On Wed, Oct 26, 2016 at 7:09 AM, Alexandre Torgue
> <alexandre.torgue@st.com> wrote:
>> Hi Bruno,
>>
>> On 10/25/2016 11:06 PM, Bruno Herrera wrote:
>>>
>>> Hi Alexandre,
>>>
>>>>
>>>> stm32f469-disco and stm32f429-eval boards use SDRAM start address
>>>> remapping
>>>> (to @0) to boost performances. A DMA translation through "dma-ranges"
>>>> property was needed for other masters than the M4 CPU.
>>>> stm32f429-disco doesn't use remapping so doesn't need this DMA
>>>> translation.
>>>> This patches moves this DMA translation definition from stm32f429 soc
>>>> file
>>>> to board files.
>>>>
>>>> Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>
>>>>
>>>> diff --git a/arch/arm/boot/dts/stm32429i-eval.dts
>>>> b/arch/arm/boot/dts/stm32429i-eval.dts
>>>> index 13c7cd2..a763c15 100644
>>>> --- a/arch/arm/boot/dts/stm32429i-eval.dts
>>>> +++ b/arch/arm/boot/dts/stm32429i-eval.dts
>>>> @@ -82,6 +82,10 @@
>>>> };
>>>> };
>>>>
>>>> + soc {
>>>> + dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>> + };
>>>> +
>>>> usbotg_hs_phy: usbphy {
>>>> #phy-cells = <0>;
>>>> compatible = "usb-nop-xceiv";
>>>
>>>
>>> Shouldn't also the peripheral dma-ranges property move to board specific
>>> too?
>>> I had this patch for while but I didn't had the time to submit:
>>
>>
>> Well spot I forgot it. Actually, discussing with Arnd ysterday on IIRC,
>> empty dma-ranges is not needed. Can you test on your side by removing
>> dma-ranges in usb node please ?
> Unfortunately will take a time for me to set up this environment on
> the STM32F4-EVAL board.
> And on the discovery boards we dont have this scenario. That was the
> main reason I did not submit the patch right away.
> My conclusion and I might be wrong but is based on the my tests with
> SDIO device at STM32F469I-DISCO board.
>
> I started this issue as discussion at ST Forum but Maxime gave me the hint.
>
> https://my.st.com/public/STe2ecommunities/mcu/Lists/cortex_mx_stm32/Flat.aspx?RootFolder=https%3a%2f%2fmy%2est%2ecom%2fpublic%2fSTe2ecommunities%2fmcu%2fLists%2fcortex_mx_stm32%2fDMA2%20and%20SYSCFG_MEMRMP%20relationship&FolderCTID=0x01200200770978C69A1141439FE559EB459D7580009C4E14902C3CDE46A77F0FFD06506F5B¤tviews=44
>
>> I will push a v2 by removing empty dma-ranges if tests are ok in your side.
>
> From my understating/conclusion is: when empty property(dma-ranges) is
> the device node, the mapping will be taken in consideration when using
> DMA otherwise the mapping is ignored.
> And in the SDIO case it is needed for DEV->MEM(SDRAM) and
> MEM(SDRAM)->DEV. If it is not the case for the devices in question so
> I suppose it can work without the property.
For sure translation has to be done but I'm not sure that an empty
"dma-ranges" is needed in device node to activate it. For Ethernet empty
"dma-ranges" is not needed. I will try with usb.
alex
>
>>
>> Thanks in advance
>> Alex
>>
>>
>>>
>>> Author: Bruno Herrera <bruherrera@gmail.com>
>>> Date: Sun Oct 16 14:50:00 2016 -0200
>>>
>>> ARM: DT: STM32: Use dma-ranges property per board not at dtsi file
>>>
>>> diff --git a/arch/arm/boot/dts/stm32429i-eval.dts
>>> b/arch/arm/boot/dts/stm32429i-eval.dts
>>> index 6bfc595..2a22a82 100644
>>> --- a/arch/arm/boot/dts/stm32429i-eval.dts
>>> +++ b/arch/arm/boot/dts/stm32429i-eval.dts
>>> @@ -52,6 +52,10 @@
>>> model = "STMicroelectronics STM32429i-EVAL board";
>>> compatible = "st,stm32429i-eval", "st,stm32f429";
>>>
>>> + soc {
>>> + dma-ranges = <0xC0000000 0x0 0x10000000>;
>>> + };
>>> +
>>> chosen {
>>> bootargs = "root=/dev/ram rdinit=/linuxrc";
>>> stdout-path = "serial0:115200n8";
>>> @@ -96,6 +100,7 @@
>>>
>>> ðernet0 {
>>> status = "okay";
>>> + dma-ranges;
>>> pinctrl-0 = <ðernet0_mii>;
>>> pinctrl-names = "default";
>>> phy-mode = "mii-id";
>>> @@ -116,6 +121,7 @@
>>> };
>>>
>>> &usbotg_hs {
>>> + dma-ranges;
>>> dr_mode = "host";
>>> phys = <&usbotg_hs_phy>;
>>> phy-names = "usb2-phy";
>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>> index 7d624a2..697a133 100644
>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>> @@ -59,7 +59,6 @@
>>> };
>>>
>>> soc {
>>> - dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>
>>> timer2: timer at 40000000 {
>>> compatible = "st,stm32-timer";
>>> @@ -472,13 +471,11 @@
>>> st,syscon = <&syscfg 0x4>;
>>> snps,pbl = <8>;
>>> snps,mixed-burst;
>>> - dma-ranges;
>>> status = "disabled";
>>> };
>>>
>>> usbotg_hs: usb at 40040000 {
>>> compatible = "snps,dwc2";
>>> - dma-ranges;
>>> reg = <0x40040000 0x40000>;
>>> interrupts = <77>;
>>> clocks = <&rcc 0 29>;
>>>
>>>
>>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>>> index 0596d60..3a1cfdd 100644
>>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>>> @@ -59,8 +59,6 @@
>>>> };
>>>>
>>>> soc {
>>>> - dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>> -
>>>> timer2: timer at 40000000 {
>>>> compatible = "st,stm32-timer";
>>>> reg = <0x40000000 0x400>;
>>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>>>> b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> index 9e73656..c2213c0 100644
>>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> @@ -64,6 +64,10 @@
>>>> aliases {
>>>> serial0 = &usart3;
>>>> };
>>>> +
>>>> + soc {
>>>> + dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>> + };
>>>> };
>>>>
>>>> &clk_hse {
>>>> --
>>>
>>>
>>>
>>> Br.,
>>> Bruno
>>>
>>
^ permalink raw reply
* [PATCH v2 3/4] arm64: arch_timer: Work around Erratum Hisilicon-161601
From: Ding Tianhong @ 2016-10-27 12:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <16d8f329-3413-11da-6f2a-8b1a95c85352@arm.com>
On 2016/10/27 18:58, Marc Zyngier wrote:
> On 27/10/16 08:34, Ding Tianhong wrote:
>> Erratum Hisilicon-161601 says that the ARM generic timer counter "has the
>> potential to contain an erroneous value when the timer value changes".
>> Accesses to TVAL (both read and write) are also affected due to the implicit counter
>> read. Accesses to CVAL are not affected.
>>
>> The workaround is to reread the system count registers until the value of the second
>> read is larger than the first one by less than 32, the system counter can be guaranteed
>> not to return wrong value twice by back-to-back read and the error value is always larger
>> than the correct one by 32. Writes to TVAL are replaced with an equivalent write to CVAL.
>>
>> The workaround is enabled if the hisilicon,erratum-161601 property is found in
>> the timer node in the device tree. This can be overridden with the
>> clocksource.arm_arch_timer.hisilicon-161601 boot parameter, which allows KVM
>> users to enable the workaround until a mechanism is implemented to
>> automatically communicate this information.
>>
>> Fix some description for fsl erratum a008585.
>>
>> v2: Significant rework based on feedback, including seperate the fsl erratum a008585
>> to another patch, update the erratum name and remove unwanted code.
>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> Documentation/arm64/silicon-errata.txt | 1 +
>> Documentation/kernel-parameters.txt | 9 ++++
>> arch/arm64/include/asm/arch_timer.h | 28 ++++++++++-
>> drivers/clocksource/Kconfig | 14 +++++-
>> drivers/clocksource/arm_arch_timer.c | 88 ++++++++++++++++++++++++++--------
>> 5 files changed, 118 insertions(+), 22 deletions(-)
>>
>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
>> index 405da11..70c5d5e 100644
>> --- a/Documentation/arm64/silicon-errata.txt
>> +++ b/Documentation/arm64/silicon-errata.txt
>> @@ -63,3 +63,4 @@ stable kernels.
>> | Cavium | ThunderX SMMUv2 | #27704 | N/A |
>> | | | | |
>> | Freescale/NXP | LS2080A/LS1043A | A-008585 | FSL_ERRATUM_A008585 |
>> +| Hisilicon | Hip05/Hip06/Hip07 | #161601 | HISILICON_ERRATUM_161601|
>
> I've already commented on the alignment. Please read my initial review.
>
It sees misunderstood, fix it this time.
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index 6fa1d8a..735b4b6 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -707,6 +707,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>> erratum. If unspecified, the workaround is
>> enabled based on the device tree.
>>
>> + clocksource.arm_arch_timer.hisilicon-161601=
>> + [ARM64]
>> + Format: <bool>
>> + Enable/disable the workaround of Hisilicon
>> + erratum 161601. This can be useful for KVM
>> + guests, if the guest device tree doesn't show the
>> + erratum. If unspecified, the workaround is
>> + enabled based on the device tree.
>> +
>> clearcpuid=BITNUM [X86]
>> Disable CPUID feature X for the kernel. See
>> arch/x86/include/asm/cpufeatures.h for the valid bit
>> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
>> index 118719d8..49b3041 100644
>> --- a/arch/arm64/include/asm/arch_timer.h
>> +++ b/arch/arm64/include/asm/arch_timer.h
>> @@ -29,7 +29,7 @@
>>
>> #include <clocksource/arm_arch_timer.h>
>>
>> -#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585)
>> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>> extern struct static_key_false arch_timer_read_ool_enabled;
>> #define needs_timer_erratum_workaround() \
>> static_branch_unlikely(&arch_timer_read_ool_enabled)
>> @@ -65,11 +65,35 @@ extern struct arch_timer_erratum_workaround *erratum_workaround;
>> _new; \
>> })
>>
>> +
>> +
>> +/*
>> + * The number of retries is an arbitrary value well beyond the highest number
>> + * of iterations the loop has been observed to take.
>> + * Verify whether the value of the second read is larger than the first by
>> + * less than 32 is the only way to confirm the value is correct, the system
>> + * counter can be guaranteed not to return wrong value twice by back-to-back read
>> + * and the error value is always larger than the correct one by 32.
>> + */
>> +#define __hisi_161601_read_reg(reg) ({ \
>> + u64 _old, _new; \
>> + int _retries = 200; \
>
> Please document how this value was found (either in the code or in the
> commit message).
It really difficult to give the accurate standard, theoretically the error should not happened
twice together, so maybe 2 is enough here, I just give a arbitrary value.
>
>> + \
>> + do { \
>> + _old = read_sysreg(reg); \
>> + _new = read_sysreg(reg); \
>> + _retries--; \
>> + } while (unlikely((_new - _old) >> 5) && _retries); \
>> + \
>> + WARN_ON_ONCE(!_retries); \
>> + _new; \
>> +})
>
> Same remark as in the previous patch.
>
I think the sentence *Verify whether the value of the second read is larger than the first by
less than 32 is the only way to confirm the value is correct* could explain why should *(_new - _old) >> 5*.
it is the same as (_new - _old)/32, also mean the _new should never bigger than _old more than 32.
>> +
>> #define arch_timer_reg_read_stable(reg) \
>> ({ \
>> u64 _val; \
>> if (needs_timer_erratum_workaround()) \
>> - _val = erratum_workaround->read_##reg(); \
>> + _val = erratum_workaround->read_##reg();\
>> else \
>> _val = read_sysreg(reg); \
>> _val; \
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index 8a753fd..4aafb6a 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -312,8 +312,20 @@ config FSL_ERRATUM_A008585
>> help
>> This option enables a workaround for Freescale/NXP Erratum
>> A-008585 ("ARM generic timer may contain an erroneous
>> - value"). The workaround will only be active if the
>> + value"). The workaround will be active if the
>> fsl,erratum-a008585 property is found in the timer node.
>> + This can be overridden with the clocksource.arm_arch_timer.fsl-a008585
>> + boot parameter.
>
> Move this hunk to the previous patch.
>
>> +
>> +config HISILICON_ERRATUM_161601
>> + bool "Workaround for Hisilicon Erratum 161601"
>> + default y
>> + depends on ARM_ARCH_TIMER && ARM64
>> + help
>> + This option enables a workaround for Hisilicon Erratum
>> + 161601. The workaround will be active if the hisilicon,erratum-161601
>> + property is found in the timer node. This can be overridden with
>> + the clocksource.arm_arch_timer.hisilicon-161601 boot parameter.
>>
>> config ARM_GLOBAL_TIMER
>> bool "Support for the ARM global timer" if COMPILE_TEST
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
>> index e4f7fa1..89f1895 100644
>> --- a/drivers/clocksource/arm_arch_timer.c
>> +++ b/drivers/clocksource/arm_arch_timer.c
>> @@ -94,13 +94,14 @@ early_param("clocksource.arm_arch_timer.evtstrm", early_evtstrm_cfg);
>> * Architected system timer support.
>> */
>>
>> -#ifdef CONFIG_FSL_ERRATUM_A008585
>> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>> struct arch_timer_erratum_workaround *erratum_workaround = NULL;
>>
>> DEFINE_STATIC_KEY_FALSE(arch_timer_read_ool_enabled);
>> EXPORT_SYMBOL_GPL(arch_timer_read_ool_enabled);
>> +#endif
>>
>> -
>> +#ifdef CONFIG_FSL_ERRATUM_A008585
>> static u32 fsl_a008585_read_cntp_tval_el0(void)
>> {
>> return __fsl_a008585_read_reg(cntp_tval_el0);
>> @@ -139,6 +140,45 @@ static int __init early_fsl_a008585_cfg(char *buf)
>> early_param("clocksource.arm_arch_timer.fsl-a008585", early_fsl_a008585_cfg);
>> #endif /* CONFIG_FSL_ERRATUM_A008585 */
>>
>> +#ifdef CONFIG_HISILICON_ERRATUM_161601
>> +static u32 hisi_161601_read_cntp_tval_el0(void)
>> +{
>> + return __hisi_161601_read_reg(cntp_tval_el0);
>> +}
>> +
>> +static u32 hisi_161601_read_cntv_tval_el0(void)
>> +{
>> + return __hisi_161601_read_reg(cntv_tval_el0);
>> +}
>> +
>> +static u64 hisi_161601_read_cntvct_el0(void)
>> +{
>> + return __hisi_161601_read_reg(cntvct_el0);
>> +}
>> +
>> +static struct arch_timer_erratum_workaround arch_timer_hisi_161601 = {
>> + .read_cntp_tval_el0 = hisi_161601_read_cntp_tval_el0,
>> + .read_cntv_tval_el0 = hisi_161601_read_cntv_tval_el0,
>> + .read_cntvct_el0 = hisi_161601_read_cntvct_el0,
>> +};
>> +
>> +static int __init early_hisi_161601_cfg(char *buf)
>> +{
>> + int ret;
>> + bool val;
>> +
>> + ret = strtobool(buf, &val);
>> + if (ret)
>> + return ret;
>> +
>> + if (val)
>> + erratum_workaround = &arch_timer_hisi_161601;
>> +
>> + return 0;
>> +}
>> +early_param("clocksource.arm_arch_timer.hisilicon-161601", early_hisi_161601_cfg);
>> +#endif /* CONFIG_HISILICON_ERRATUM_161601 */
>> +
>> static __always_inline
>> void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val,
>> struct clock_event_device *clk)
>> @@ -288,8 +328,8 @@ static __always_inline void set_next_event(const int access, unsigned long evt,
>> arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
>> }
>>
>> -#ifdef CONFIG_FSL_ERRATUM_A008585
>> -static __always_inline void fsl_a008585_set_next_event(const int access,
>> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>> +static __always_inline void erratum_set_next_event(const int access,
>> unsigned long evt, struct clock_event_device *clk)
>> {
>> unsigned long ctrl;
>> @@ -307,20 +347,20 @@ static __always_inline void fsl_a008585_set_next_event(const int access,
>> arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
>> }
>>
>> -static int fsl_a008585_set_next_event_virt(unsigned long evt,
>> +static int erratum_set_next_event_virt(unsigned long evt,
>> struct clock_event_device *clk)
>> {
>> - fsl_a008585_set_next_event(ARCH_TIMER_VIRT_ACCESS, evt, clk);
>> + erratum_set_next_event(ARCH_TIMER_VIRT_ACCESS, evt, clk);
>> return 0;
>> }
>>
>> -static int fsl_a008585_set_next_event_phys(unsigned long evt,
>> +static int erratum_set_next_event_phys(unsigned long evt,
>> struct clock_event_device *clk)
>> {
>> - fsl_a008585_set_next_event(ARCH_TIMER_PHYS_ACCESS, evt, clk);
>> + erratum_set_next_event(ARCH_TIMER_PHYS_ACCESS, evt, clk);
>> return 0;
>> }
>> -#endif /* CONFIG_FSL_ERRATUM_A008585 */
>> +#endif
>>
>> static int arch_timer_set_next_event_virt(unsigned long evt,
>> struct clock_event_device *clk)
>> @@ -350,16 +390,16 @@ static int arch_timer_set_next_event_phys_mem(unsigned long evt,
>> return 0;
>> }
>>
>> -static void fsl_a008585_set_sne(struct clock_event_device *clk)
>> +static void erratum_set_sne(struct clock_event_device *clk)
>> {
>> -#ifdef CONFIG_FSL_ERRATUM_A008585
>> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>> if (!static_branch_unlikely(&arch_timer_read_ool_enabled))
>> return;
>>
>> if (arch_timer_uses_ppi == VIRT_PPI)
>> - clk->set_next_event = fsl_a008585_set_next_event_virt;
>> + clk->set_next_event = erratum_set_next_event_virt;
>> else
>> - clk->set_next_event = fsl_a008585_set_next_event_phys;
>> + clk->set_next_event = erratum_set_next_event_phys;
>> #endif
>
> This should be in the previous patch as well, as it only messes with the
> FSL erratum.
>
Ok.
>> }
>>
>> @@ -392,7 +432,7 @@ static void __arch_timer_setup(unsigned type,
>> BUG();
>> }
>>
>> - fsl_a008585_set_sne(clk);
>> + erratum_set_sne(clk);
>> } else {
>> clk->features |= CLOCK_EVT_FEAT_DYNIRQ;
>> clk->name = "arch_mem_timer";
>> @@ -612,7 +652,7 @@ static void __init arch_counter_register(unsigned type)
>>
>> clocksource_counter.archdata.vdso_direct = true;
>>
>> -#ifdef CONFIG_FSL_ERRATUM_A008585
>> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>> /*
>> * Don't use the vdso fastpath if errata require using
>> * the out-of-line counter accessor.
>> @@ -899,12 +939,22 @@ static int __init arch_timer_of_init(struct device_node *np)
>> arch_timer_c3stop = !of_property_read_bool(np, "always-on");
>>
>> #ifdef CONFIG_FSL_ERRATUM_A008585
>> - if (!erratum_workaround && of_property_read_bool(np, "fsl,erratum-a008585"))
>> + if (!erratum_workaround && of_property_read_bool(np, "fsl,erratum-a008585")) {
>> erratum_workaround = &arch_timer_fsl_a008585;
>> + if (erratum_workaround) {
>
> Brilliant!
>
>> + static_branch_enable(&arch_timer_read_ool_enabled);
>> + pr_info("Enabling workaround for FSL erratum A-008585\n");
>> + }
>> + }
>> +#endif
>>
>> - if (erratum_workaround) {
>> - static_branch_enable(&arch_timer_read_ool_enabled);
>> - pr_info("Enabling workaround for FSL erratum A-008585\n");
>> +#ifdef CONFIG_HISILICON_ERRATUM_161601
>> + if (!erratum_workaround && of_property_read_bool(np, "hisilicon,erratum-161601")) {
>> + erratum_workaround = &arch_timer_hisi_161601;
>> + if (erratum_workaround) {
>> + static_branch_enable(&arch_timer_read_ool_enabled);
>> + pr_info("Enabling workaround for HISILICON erratum 161601\n");
>> + }
>> }
>> #endif
>
> Do you see a pattern here? Surely you can factor a bit of that code (and
> remove the nonsensical bits).
>
Yes, found it, try to fix it.
Thanks.
Ding
> Thanks,
>
> M.
>
^ permalink raw reply
* [PATCH v3] ARM: davinci: da8xx: Fix some redefined symbol warnings
From: Sekhar Nori @ 2016-10-27 11:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7026f012-ffce-cc28-85be-9a5cd26de60f@ti.com>
On Thursday 27 October 2016 05:13 PM, Sekhar Nori wrote:
> On Wednesday 26 October 2016 06:09 PM, Alexandre Bailon wrote:
>> Some macro for DA8xx CFGCHIP are defined in usb-davinci.h,
>> but da8xx-cfgchip.h intend to replace them.
>> The usb-da8xx.c is using both headers, causing redefined symbol warnings.
>
> Looks like this is not true for v4.9-rc2 and so I don't see any warnings
Ah, just noticed that _this_ is the patch that introduces
da8xx-cfgchip.h into usb-da8xx.c. So this is the patch that introduces
the warnings (and fixes them).
I can queue this for v4.10 (with Greg's ack) if you change the
description to make it about cleaning up duplicated defines between
da8xx-cfgchip.h and usb-davinci.h and not talk about "redefined symbol
warnings".
Also, when adding a header file, can you please keep it sorted in
alphabetical order.
Thanks,
Sekhar
^ permalink raw reply
* [PATCH] ARM: shmobile: select errata 798181 for SoCs with CA15 cores
From: Simon Horman @ 2016-10-27 11:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANqRtoTJHrXK4NBWWDShoF1RnxpBhheDnvsfeyDO6cvLdbOE=A@mail.gmail.com>
On Thu, Oct 27, 2016 at 04:39:24PM +0900, Magnus Damm wrote:
> Hi Geert and Simon,
>
> On Thu, Oct 27, 2016 at 4:33 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > Hi Magnus,
> >
> > On Thu, Oct 27, 2016 at 9:28 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> >> On Thu, Oct 27, 2016 at 4:00 PM, Simon Horman
> >> <horms+renesas@verge.net.au> wrote:
> >>> Select ARM errata 798181 on SoCs cores affected CA15 cores.
> >>>
> >>> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> >>> ---
> >>> arch/arm/mach-shmobile/Kconfig | 6 ++++++
> >>> 1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> >>> index c48be1d332ed..6fbd9b7d2d67 100644
> >>> --- a/arch/arm/mach-shmobile/Kconfig
> >>> +++ b/arch/arm/mach-shmobile/Kconfig
> >>> @@ -60,6 +60,7 @@ config ARCH_R7S72100
> >>> config ARCH_R8A73A4
> >>> bool "R-Mobile APE6 (R8A73A40)"
> >>> select ARCH_RMOBILE
> >>> + select ARM_ERRATA_798181 if SMP
> >>> select RENESAS_IRQC
> >>>
> >>> config ARCH_R8A7740
> >>> @@ -70,6 +71,7 @@ config ARCH_R8A7740
> >>> config ARCH_R8A7743
> >>> bool "RZ/G1M (R8A77430)"
> >>> select ARCH_RCAR_GEN2
> >>> + select ARM_ERRATA_798181 if SMP
> >>>
> >>> config ARCH_R8A7778
> >>> bool "R-Car M1A (R8A77781)"
> >>> @@ -82,20 +84,24 @@ config ARCH_R8A7779
> >>> config ARCH_R8A7790
> >>> bool "R-Car H2 (R8A77900)"
> >>> select ARCH_RCAR_GEN2
> >>> + select ARM_ERRATA_798181 if SMP
> >>> select I2C
> >>
> >> Thanks for your help.
> >>
> >> I'm probably misunderstanding what this patch does and how the errata
> >> effects the system, but the commit message says CA15 cores. The above
> >> R-Car Gen1 and r8a7740 SoCs are not using CA15 - instead they use CA9.
> >> Not sure if the errata still applies though.
> >
> > Please don't become misled by the @@ context ;-)
>
> Ouch, right... Sorry for the noise. The patch looks good, please proceed.
Thanks, I have queued this up.
^ permalink raw reply
* Pinctrl nodes missing for USB
From: Anand Moon @ 2016-10-27 11:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161026175827.GA10224@kozik-lap>
Hi Krzysztof,
On 26 October 2016 at 23:28, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Wed, Oct 26, 2016 at 11:15:51PM +0530, Anand Moon wrote:
>> Hi Krzysztof
>>
>> On 26 October 2016 at 22:23, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>> > On Wed, Oct 26, 2016 at 05:56:54PM +0530, Anand Moon wrote:
>> >> Hi All,
>> >>
>> >> I have tried to enable CONFIG_DEBUG_PINCTRL=y on Odroid XU4.
>> >> Just to try to understand the feature.
>> >> Is this feature suppoted for USB nodes.
>> >>
>> >> Below is the output of failed to pase pinctrl for USB nodes via dts.
>> >
>> > I do not see any question here...
>> >
>> > Anyway the devices not instantiated from DT will have such warning and
>> > USB devices are not present in DT, from obvious reasons... However what
>> > surprises me is why pinctrl_dt_to_map() was called for USB devices?
>> >
>> > Best regards,
>> > Krzysztof
>> >
>> [snip]
>>
>> Sorry. I was just referring HK odroidxu3 dts for reference for dwc3 controller.
>>
>> https://github.com/hardkernel/linux/blob/odroidxu3-3.10.y/arch/arm/boot/dts/exynos5422-odroidxu3.dts#L525
>>
>> I am just trying to understand if such a configuration possible for
>> dwc3 controllers.
>
> What do you mean by "configuration"? Which configuration?
I am just elaborating what I have understood below.
>
> Best regards,
> Krzysztof
>
Adding Vivek Gautam +
Apologize for my poor English and explanation of what I am trying to
work on or understand the feature related to dwc3
For some time I am trying to figure out the performance issue of USB
3.0 on Odroid XU4 boards.
[1] https://lkml.org/lkml/2015/2/2/259
Following above patch helps registration of USB storage device to
"Super-Speed" ie
root at odroidcsh:/usr/src/odroidxu3-4.y-devel# lsusb -t
/: Bus 06.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M
/: Bus 05.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 480M
|__ Port 1: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 480M
/: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M
|__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/2p, 5000M
|__ Port 1: Dev 3, If 0, Class=Mass Storage, Driver=usb-storage, 5000M
|__ Port 2: Dev 4, If 0, Class=Mass Storage, Driver=usb-storage, 5000M
/: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 480M
|__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/2p, 480M
/: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=exynos-ohci/3p, 12M
/: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=exynos-ehci/3p, 480M
But the performance is not good enough to work on the storage device.
we cannot compile whole kernel or source code on this drive attached.
So did some investigation on this by comparing the dts nodes and driver.
I was looking into some logic to do some "gpio_reset" which will help
reset the driver.
But after studying the driver code of OdroidXU4 Hardkernel,
I tried to understand the requirement of gpio pin controlled by pinctrl.
Sorry I am not an expert in the internal of the the pinctrl and
internal gpio bus.
So I have just for my understanding created this small patch to help
elaborate this feature.
--------------------------------------------------------------------------------------------
root at odroidcsh:/usr/src/odroidxu3-4.y-devel# git diff
arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
index 246d298..03e90b6 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
@@ -560,6 +560,24 @@
samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
samsung,pin-drv = <EXYNOS5420_PIN_DRV_LV1>;
};
+
+ b_sess0_irq: b-sess0-irq {
+ samsung,pins = "gpx3-5";
+ samsung,pin-function = <0xf>;
+ samsung,pin-pud = <0>;
+ };
+
+ b_sess1_irq: b-sess1-irq {
+ samsung,pins = "gpx3-4";
+ samsung,pin-function = <0xf>;
+ samsung,pin-pud = <0>;
+ };
+
+ id2_irq: id2-irq {
+ samsung,pins = "gpx1-1";
+ samsung,pin-function = <0xf>;
+ samsung,pin-pud = <0>;
+ };
};
&pinctrl_1 {
@@ -569,6 +587,12 @@
samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
samsung,pin-drv = <EXYNOS5420_PIN_DRV_LV1>;
};
+
+ id1_irq: id1-irq {
+ samsung,pins = "gpc1-0";
+ samsung,pin-function = <0xf>;
+ samsung,pin-pud = <3>;
+ };
};
&tmu_cpu0 {
@@ -604,11 +628,23 @@
/* usbdrd_dwc3_1 mode customized in each board */
&usbdrd3_0 {
+ samsung,bsess-gpio = <&gpx3 5 0xf>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&b_sess0_irq>;
+ samsung,id-gpio = <&gpc1 0 0xf>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&id1_irq>;
vdd33-supply = <&ldo9_reg>;
vdd10-supply = <&ldo11_reg>;
};
&usbdrd3_1 {
+ samsung,bsess-gpio = <&gpx3 4 0xf>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&b_sess1_irq>;
+ samsung,id-gpio = <&gpx1 1 0xf>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&id2_irq>;
vdd33-supply = <&ldo9_reg>;
vdd10-supply = <&ldo11_reg>;
};
--------------------------------------------------------------------------------------------
Here is the core logic as I understood out of the driver code from
OdroidXU3 Hardkernel.
so we have samsung,bsess-gpio and samsung,id-gpio two gpio pins control
by the exynos-dwc3 driver in the Odroid Hardkernel
# drivers/usb/dwc3/dwc3-exynos.c
tries to register interrupts on these gpio pin to control the flow via gpio-irq.
Also It monitor the vbus controller changes
Below irq to monitor id-gpio thread.
https://github.com/hardkernel/linux/blob/odroidxu3-3.10.y/drivers/usb/dwc3/dwc3-exynos.c#L217
Below irq to monitor bsess-gpio thread.
https://github.com/hardkernel/linux/blob/odroidxu3-3.10.y/drivers/usb/dwc3/dwc3-exynos.c#L230
Once again I am not an expert in this configuration, I am just trying
to map the feature and the code.
Please let me know If my understanding is wrong or the feature should
work little bit different.
I am poor in English to explain technically details.
If any body have any other input on this please let me know.
-Best Regards
Anand Moon
^ permalink raw reply related
* [PATCH v3] ARM: davinci: da8xx: Fix some redefined symbol warnings
From: Sekhar Nori @ 2016-10-27 11:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477485542-10555-2-git-send-email-abailon@baylibre.com>
On Wednesday 26 October 2016 06:09 PM, Alexandre Bailon wrote:
> Some macro for DA8xx CFGCHIP are defined in usb-davinci.h,
> but da8xx-cfgchip.h intend to replace them.
> The usb-da8xx.c is using both headers, causing redefined symbol warnings.
Looks like this is not true for v4.9-rc2 and so I don't see any warnings
in v4.9-rc2. This is probably introduced due to some changes you are
doing for v4.10 and so it is not a v4.9-rc candidate.
> Remove the macro and update the board files to use da8xx-cfgchip.h
This patch should probably be folded into the patch which actually
introduces da8xx-cfgchip.h into usb-da8xx.c
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Thanks,
Sekhar
^ permalink raw reply
* [PATCH] ASoC: hdmi-codec: Fix hdmi_of_xlate_dai_name when #sound-dai-cells = <0>
From: Jon Medhurst (Tixy) @ 2016-10-27 11:42 UTC (permalink / raw)
To: linux-arm-kernel
If a DAI specifies "#sound-dai-cells = <0>" in device-tree then
hdmi_of_xlate_dai_name() will be called with zero args, which it isn't
implemented to cope with. The resulting use of an uninitialised variable
for the id will usually result in an error like:
asoc-simple-card sound: parse error -11
asoc-simple-card: probe of sound failed with error -11
Fix this by using and id of zero if no arg is provided.
Fixes: 9731f82d6016 ("ASoC: hdmi-codec: enable multi probe for same device")
Signed-off-by: Jon Medhurst <tixy@linaro.org>
---
Jyri, FYI, I believe this issue broke your commit df0bd1e8f3c508 ("ARM: dts:
am335x-boneblack: Add HDMI audio support")
sound/soc/codecs/hdmi-codec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index b904492..6661ace 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -364,7 +364,7 @@ static int hdmi_of_xlate_dai_name(struct snd_soc_component *component,
struct of_phandle_args *args,
const char **dai_name)
{
- int id = args->args[0];
+ int id = args->args_count ? args->args[0] : 0;
if (id < ARRAY_SIZE(hdmi_dai_name)) {
*dai_name = hdmi_dai_name[id];
--
2.1.4
^ permalink raw reply related
* [PATCH 4/5] ARM: davinci: enable LEDs default-on trigger in default config
From: Sekhar Nori @ 2016-10-27 11:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477075018-20176-5-git-send-email-david@lechnology.com>
On Saturday 22 October 2016 12:06 AM, David Lechner wrote:
> The LEDs default-on trigger is nice to have. For example, it can be used
> to configure a LED as a power indicator.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
> arch/arm/configs/davinci_all_defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
> index 9d7f0bc..e380743 100644
> --- a/arch/arm/configs/davinci_all_defconfig
> +++ b/arch/arm/configs/davinci_all_defconfig
> @@ -181,6 +181,7 @@ CONFIG_LEDS_GPIO=m
> CONFIG_LEDS_TRIGGERS=y
> CONFIG_LEDS_TRIGGER_TIMER=m
> CONFIG_LEDS_TRIGGER_HEARTBEAT=m
> +CONFIG_LEDS_TRIGGER_DEFAULT_ON=y
Can this be module like rest of the triggers? It will come on later, but
not sure if you care about the difference that much. Having it a module
will be better for those boards that don't need it.
Thanks,
Sekhar
^ permalink raw reply
* [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz
From: Mugunthan V N @ 2016-10-27 11:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024060226.4170-1-mugunthanvnm@ti.com>
On Monday 24 October 2016 11:32 AM, Mugunthan V N wrote:
> Increase ADC reference clock from 3MHz to 24MHz so that the
> sampling rates goes up from 100K samples per second to 800K
> samples per second on AM335x and AM437x SoC.
>
> Also increase opendelay for touchscreen configuration to
> equalize the increase in ADC reference clock frequency,
> which results in the same amount touch events reported via
> evtest on AM335x GP EVM.
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Found issues with touchscreen usage with GUI on my AM335x GP EVM. Fixed
with change the "ti,charge-delay" DT property based on change in ADC
clocks. Will be sending v2 patch soon.
Regards
Mugunthan V N
^ permalink raw reply
* [PATCH 5/5] ARM: dts: Add LEGO MINDSTORTMS EV3 dts
From: Sekhar Nori @ 2016-10-27 11:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <38a3a10c-a269-6a9f-305c-7d38ec9b839b@lechnology.com>
On Thursday 27 October 2016 07:00 AM, David Lechner wrote:
> On 10/24/2016 06:58 AM, Sekhar Nori wrote:
>> On Saturday 22 October 2016 12:06 AM, David Lechner wrote:
>>> This adds a device tree definition file for LEGO MINDSTORMS EV3.
>>
>> Thanks for the patch!
>>
>>>
>>> What is working:
>>>
>>> * Pin muxing
>>> * MicroSD card reader
>>> * UART on input port 1
>>>
>>> What is partially working:
>>>
>>> * Buttons - working after GPIO fix
>>> * LEDs - working after GPIO fix
>>> * Poweroff/reset - working after GPIO fix
>>
>> Is the GPIO fix something that will go in v4.9-rc cycle ?
>>
>
> FYI, the fix is in linux-gpio/fixes now.
Thanks, I added the patch to my 'acked' branch which has stuff useful
for testing on davinci but not going through my tree. It gets merged
into my master branch so that branch can be used for all mach-davinci
development.
Thanks,
Sekhar
^ 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