* Re: [PATCH] soc: qcom: Add SoC info driver
From: Arnd Bergmann @ 2016-10-27 13:41 UTC (permalink / raw)
To: Imran Khan
Cc: andy.gross, David Brown, Rob Herring, Mark Rutland,
open list:ARM/QUALCOMM SUPPORT, open list:ARM/QUALCOMM SUPPORT,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <fd34e299-a3c2-e981-a92e-b61638a98e5f@codeaurora.org>
On Thursday, October 27, 2016 6:40:27 PM CEST Imran Khan wrote:
> On 10/26/2016 8:16 PM, Arnd Bergmann wrote:
> > On Wednesday, October 26, 2016 7:42:08 PM CEST Imran Khan wrote:
> >> On 10/26/2016 7:35 PM, Arnd Bergmann wrote:
> >>>>> As we are talking about generic soc_device_attribute fields, I was hoping that
> >>>>> having a vendor field would be helpful as along with family it would provide
> >>>>> a more thorough information. Also as more than one foundries may be used for
> >>>>> a soc, can we have a field say foundry_id to provide this information.
> >>> My first feeling is that this 'vendor' information can should be
> >>> derived from the family. It's also not clear what would happen
> >>> to this when a company gets bought. E.g. the Oxnas product family
> >>> was subsequently owned by Oxford, PLX, Avago and Broadcom, and the
> >>> mxs family was Sigmatel, Freescale, now NXP and might soon be
> >>> Qualcomm. What would you put in there in this case?
> >>
> >> Okay, not having vendor field is fine for me. Could you also suggest
> >> something about the foundry_id field.
> >
> > This one seems more well-defined, so it's probably ok to add. What
> > would be the use case of reading this? Would you want to read it
> > just from user space or also from the kernel?
> >
>
> As of now the use case I can think of, only involve reading this from user
> space. For example for the same soc, coming from different foundries with
> different manufacturing process, we may have a situation where some inconsistent
> h/w behavior is being observed only on parts received from a certain foundry
> and in those cases this information may help in segregation of problematic socs
> and may also be used in testing these socs under a different set of settings like
> voltage, frequency etc.
>
> > Maybe this can be combined with a manufacturing process, which probably
> > falls into a similar category, so we could have something like
> > "TSMC 28ULP" as a string in there.
> >
>
> Yes. Having a manufacturing process as part of foundry-id can provide a more
> thorough information.
Ok, sounds good. Let's do it like this. We can always add support for
in-kernel matching of this string if needed later.
Arnd
^ permalink raw reply
* Re: [PATCH v2 2/4] dt-bindings: Add TI SCI PM Domains
From: Dave Gerlach @ 2016-10-27 13:15 UTC (permalink / raw)
To: Rob Herring, Kevin Hilman, Ulf Hansson, Jon Hunter
Cc: Nishanth Menon, devicetree@vger.kernel.org,
linux-pm@vger.kernel.org, Keerthy, Santosh Shilimkar,
Rafael J . Wysocki, linux-kernel@vger.kernel.org, Tero Kristo,
Russell King, Sudeep Holla, linux-arm-kernel@lists.infradead.org
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@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
* Re: [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: David Wu
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, heiko-4mtYJXux2i+zQB+pC5nmwQ,
dianders-F7+t8E8rja9g9hUCZPvPmw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1477125822-30644-1-git-send-email-david.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 600 bytes --]
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-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
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"
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 200 bytes --]
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* Re: [PATCH] soc: qcom: Add SoC info driver
From: Imran Khan @ 2016-10-27 13:10 UTC (permalink / raw)
To: Arnd Bergmann
Cc: andy.gross, David Brown, Rob Herring, Mark Rutland,
open list:ARM/QUALCOMM SUPPORT, open list:ARM/QUALCOMM SUPPORT,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <4173870.AkctyO62lp@wuerfel>
On 10/26/2016 8:16 PM, Arnd Bergmann wrote:
> On Wednesday, October 26, 2016 7:42:08 PM CEST Imran Khan wrote:
>> On 10/26/2016 7:35 PM, Arnd Bergmann wrote:
>>>>> As we are talking about generic soc_device_attribute fields, I was hoping that
>>>>> having a vendor field would be helpful as along with family it would provide
>>>>> a more thorough information. Also as more than one foundries may be used for
>>>>> a soc, can we have a field say foundry_id to provide this information.
>>> My first feeling is that this 'vendor' information can should be
>>> derived from the family. It's also not clear what would happen
>>> to this when a company gets bought. E.g. the Oxnas product family
>>> was subsequently owned by Oxford, PLX, Avago and Broadcom, and the
>>> mxs family was Sigmatel, Freescale, now NXP and might soon be
>>> Qualcomm. What would you put in there in this case?
>>
>> Okay, not having vendor field is fine for me. Could you also suggest
>> something about the foundry_id field.
>
> This one seems more well-defined, so it's probably ok to add. What
> would be the use case of reading this? Would you want to read it
> just from user space or also from the kernel?
>
As of now the use case I can think of, only involve reading this from user
space. For example for the same soc, coming from different foundries with
different manufacturing process, we may have a situation where some inconsistent
h/w behavior is being observed only on parts received from a certain foundry
and in those cases this information may help in segregation of problematic socs
and may also be used in testing these socs under a different set of settings like
voltage, frequency etc.
> Maybe this can be combined with a manufacturing process, which probably
> falls into a similar category, so we could have something like
> "TSMC 28ULP" as a string in there.
>
Yes. Having a manufacturing process as part of foundry-id can provide a more
thorough information.
> Arnd
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a\nmember of the Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* RE: [PATCH V2 08/10] watchdog: da9062/61: watchdog driver
From: Steve Twiss @ 2016-10-27 13:10 UTC (permalink / raw)
To: Guenter Roeck
Cc: LINUX-KERNEL, LINUX-WATCHDOG, Wim Van Sebroeck, DEVICETREE,
Dmitry Torokhov, LINUX-INPUT, Rob Herring
In-Reply-To: <20161026185926.GA16142@roeck-us.net>
On 26 October 2016 19:59 Guenter Roeck wrote:
> On Wed, Oct 26, 2016 at 05:56:39PM +0100, Steve Twiss wrote:
> > From: Steve Twiss <stwiss.opensource@diasemi.com>
> >
> > +static const struct of_device_id da9062_compatible_id_table[] = {
> > + { .compatible = "dlg,da9062-watchdog", .data = &da9062_watchdog_info },
> > + { },
> > +};
> > +
> > static int da9062_wdt_probe(struct platform_device *pdev)
> > {
> > int ret;
> > struct da9062 *chip;
> > struct da9062_watchdog *wdt;
> > + const struct of_device_id *match;
> >
> > chip = dev_get_drvdata(pdev->dev.parent);
> > if (!chip)
> > return -EINVAL;
> >
> > + match = of_match_node(da9062_compatible_id_table,
> > + pdev->dev.of_node);
> > + if (!match)
> > + return -ENXIO;
> > +
> > wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
> > if (!wdt)
> > return -ENOMEM;
> >
> > wdt->hw = chip;
> >
> > - wdt->wdtdev.info = &da9062_watchdog_info;
> > + wdt->wdtdev.info = (const struct watchdog_info *)match->data;
>
> I don't see why you would need match->data or of_match_node above.
Hi Guenter,
True. I do not need to do any matching on the table any more if the assumption is
to use a fall-back compatible string for DA9061 devices (to use the da9062 device
driver).
I can erase most of that content. Including the .data = &da9062_watchdog_info.
Regards,
Steve
^ 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: airlied, architt, akpm, daniel.vetter, davem, devicetree,
dri-devel, enric.balletbo, eballetbo, galak, gregkh, heiko,
ijc+devicetree, javier, jslaby, kernel, linux-arm-kernel, linux,
linux-kernel, linux, mark.rutland, martin.donnelly, martyn.welch,
mchehab, pawel.moll, peter.senna, peter.senna, p.zabel,
thierry.reding, rmk+kernel, robh+dt, shawnguo, tiwai, treding,
ykk
Cc: Fabio Estevam
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@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@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: airlied, architt, akpm, daniel.vetter, davem, devicetree,
dri-devel, enric.balletbo, eballetbo, galak, gregkh, heiko,
ijc+devicetree, javier, jslaby, kernel, linux-arm-kernel, linux,
linux-kernel, linux, mark.rutland, martin.donnelly, martyn.welch,
mchehab, pawel.moll, peter.senna, peter.senna, p.zabel,
thierry.reding, rmk+kernel, robh+dt, shawnguo, tiwai, treding,
ykk
Cc: Fabio Estevam
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: airlied, architt, akpm, daniel.vetter, davem, devicetree,
dri-devel, enric.balletbo, eballetbo, galak, gregkh, heiko,
ijc+devicetree, javier, jslaby, kernel, linux-arm-kernel, linux,
linux-kernel, linux, mark.rutland, martin.donnelly, martyn.welch,
mchehab, pawel.moll, peter.senna, peter.senna, p.zabel,
thierry.reding, rmk+kernel, robh+dt, shawnguo, tiwai, treding,
ykk
Cc: Fabio Estevam
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: airlied, architt, akpm, daniel.vetter, davem, devicetree,
dri-devel, enric.balletbo, eballetbo, galak, gregkh, heiko,
ijc+devicetree, javier, jslaby, kernel, linux-arm-kernel, linux,
linux-kernel, linux, mark.rutland, martin.donnelly, martyn.welch,
mchehab, pawel.moll, peter.senna, peter.senna, p.zabel,
thierry.reding, rmk+kernel, robh+dt, shawnguo, tiwai, treding,
ykk
Cc: Fabio Estevam
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@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: airlied, architt, akpm, daniel.vetter, davem, devicetree,
dri-devel, enric.balletbo, eballetbo, galak, gregkh, heiko,
ijc+devicetree, javier, jslaby, kernel, linux-arm-kernel, linux,
linux-kernel, linux, mark.rutland, martin.donnelly, martyn.welch,
mchehab, pawel.moll, peter.senna, peter.senna, p.zabel,
thierry.reding, rmk+kernel, robh+dt, shawnguo, tiwai, treding,
ykk
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
* Re: [PATCH] of/platform: dynamic: Use of_platform_bus_create() to create devices
From: Rob Herring @ 2016-10-27 13:00 UTC (permalink / raw)
To: Alexander Sverdlin
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Frank Rowand,
Pantelis Antoniou, Grant Likely
In-Reply-To: <b317f693-98b5-ba1e-1184-83735a68db9b-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
On Thu, Oct 20, 2016 at 6:02 AM, Alexander Sverdlin
<alexander.sverdlin-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org> wrote:
> of_platform_notify() requires parents to have OF_POPULATED_BUS to be set
> (which is correct way to verify healthy platform bus), but uses
> of_platform_device_create() to create devices, which never sets the above
> flag. Therefore it's not possible to add platform buses via overlays.
Can you create a unit test case that has the problem.
> Use of_platform_bus_create(), which is used in non-overlay case to populate
> both buses and devices.
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> ---
> drivers/of/platform.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index af98343..d2d6e74 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -521,6 +521,7 @@ static int of_platform_notify(struct notifier_block *nb,
> struct of_reconfig_data *rd = arg;
> struct platform_device *pdev_parent, *pdev;
> bool children_left;
> + int ret;
>
> switch (of_reconfig_get_state_change(action, rd)) {
> case OF_RECONFIG_CHANGE_ADD:
> @@ -534,15 +535,15 @@ static int of_platform_notify(struct notifier_block *nb,
>
> /* pdev_parent may be NULL when no bus platform device */
> pdev_parent = of_find_device_by_node(rd->dn->parent);
> - pdev = of_platform_device_create(rd->dn, NULL,
> - pdev_parent ? &pdev_parent->dev : NULL);
> + ret = of_platform_bus_create(rd->dn, of_default_bus_match_table,
> + NULL, pdev_parent ?
> + &pdev_parent->dev : NULL, true);
I think this should be of_platform_default_populate() instead.
> of_dev_put(pdev_parent);
>
> - if (pdev == NULL) {
> + if (ret) {
> pr_err("%s: failed to create for '%s'\n",
> __func__, rd->dn->full_name);
> - /* of_platform_device_create tosses the error code */
> - return notifier_from_errno(-EINVAL);
> + return notifier_from_errno(ret);
> }
> break;
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Add Allwinner Q8 tablets hardware manager
From: Pierre-Hugues Husson @ 2016-10-27 12:57 UTC (permalink / raw)
To: Hans de Goede
Cc: Mark Rutland, devicetree, Arnd Bergmann, Greg Kroah-Hartman,
Pantelis Antoniou, Chen-Yu Tsai, Rob Herring, Maxime Ripard,
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@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@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@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
* Re: [PATCH] ARM: DT: stm32: move dma translation to board files
From: Bruno Herrera @ 2016-10-27 12:57 UTC (permalink / raw)
To: Alexandre Torgue
Cc: mark.rutland, devicetree, Arnd Bergmann, linux, >, >,
Maxime Coquelin, Lee Jones, 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@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@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@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@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@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
* Re: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from local function names
From: Rob Herring @ 2016-10-27 12:47 UTC (permalink / raw)
To: Frank Rowand
Cc: Pantelis Antoniou, Pantelis Antoniou, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1477429146-27039-7-git-send-email-frowand.list@gmail.com>
On Tue, Oct 25, 2016 at 3:58 PM, <frowand.list@gmail.com> wrote:
> From: Frank Rowand <frank.rowand@am.sony.com>
I prefer to leave the prefixes and this is getting into pointless churn.
>
> Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
> ---
> drivers/of/resolver.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 3/4] arm64: arch_timer: Work around Erratum Hisilicon-161601
From: Marc Zyngier @ 2016-10-27 12:23 UTC (permalink / raw)
To: Ding Tianhong, catalin.marinas, will.deacon, mark.rutland, oss,
devicetree, shawnguo, stuart.yoder, linux-arm-kernel, linuxarm
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
* Re: [PATCH] gpio: mcp23s08: Add option to configure pullups.
From: Linus Walleij @ 2016-10-27 12:22 UTC (permalink / raw)
To: Enric Balletbo Serra
Cc: Alexander Stein, Enric Balletbo i Serra,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-gpio@vger.kernel.org, Rob Herring
In-Reply-To: <CAFqH_53SSctH9Ms-0Zm5fhEgMtkWq_SP9VChY-QYV-qDLLj_WA@mail.gmail.com>
On Wed, Oct 26, 2016 at 11:59 AM, Enric Balletbo Serra
<eballetbo@gmail.com> wrote:
> 2016-10-24 8:13 GMT+02:00 Alexander Stein
> <alexander.stein@systec-electronic.com>:
>> On Monday 24 October 2016 02:53:31, Linus Walleij wrote:
>>> On Fri, Oct 21, 2016 at 5:00 PM, Enric Balletbo i Serra
>>>
>>> <enric.balletbo@collabora.com> wrote:
>>> > Default is without pullups, but if property is specified in DT and the bit
>>> > is set, set a pullup on GPIO-n.
>>> >
>>> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>
>>> I don't generally like this.
>>>
>
> I tried to use the same approach as used in gpio-twl4030.c because is
> what I knew before. Maybe you can point me to a better approach?
Move the whole driver to a pin control variant:
Documentation/pinctrl.txt
We just did this with the sx150x, see
commit 9e80f9064e73f9f82679884ddf8b03ac3606cf4a
"pinctrl: Add SX150X GPIO Extender Pinctrl Driver"
in linux-next.
However that is a heavy operation. And a lot of upfront work.
If it is really *JUST* this one extra parameter, it's not a super-big
deal, but I do not like to create a plethora of custom attributes
to hack around stuff we already have generic solutions for.
And if it makes sense for clients to specify the pullup, it should
definately be done on the consumer side rather than on the
producer side as these approaches do. For that approach see
commit 69d301fdd19635a39cb2b78e53fdd625b7a27924
"gpio: add DT bindings for existing consumer flags"
commit c663e5f56737757db4d0b317c510ab505f93cecb
"gpio: support native single-ended hardware drivers"
Yours,
Linus Walleij
^ permalink raw reply
* Re: [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter
From: Rob Herring @ 2016-10-27 12:21 UTC (permalink / raw)
To: Frank Rowand
Cc: Pantelis Antoniou, Pantelis Antoniou, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1477429146-27039-3-git-send-email-frowand.list@gmail.com>
On Tue, Oct 25, 2016 at 3:58 PM, <frowand.list@gmail.com> wrote:
> From: Frank Rowand <frank.rowand@am.sony.com>
Maybe some should be debug?
> Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
> ---
> drivers/of/resolver.c | 28 ----------------------------
> 1 file changed, 28 deletions(-)
>
> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
> index 4ff0220d7aa2..93a7ca0bf98c 100644
> --- a/drivers/of/resolver.c
> +++ b/drivers/of/resolver.c
> @@ -116,8 +116,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
>
> propval = kmalloc(rprop->length, GFP_KERNEL);
> if (!propval) {
> - pr_err("%s: Could not copy value of '%s'\n",
> - __func__, rprop->name);
> return -ENOMEM;
> }
I would remove the brackets in this patch rather than separately.
> memcpy(propval, rprop->value, rprop->length);
^ permalink raw reply
* Re: [PATCH] ARM: DT: stm32: move dma translation to board files
From: Alexandre Torgue @ 2016-10-27 12:21 UTC (permalink / raw)
To: Bruno Herrera
Cc: mark.rutland, devicetree, Arnd Bergmann, linux, >, >,
Maxime Coquelin, Lee Jones, 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@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@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@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
* Re: [RFC PATCH 01/13] of: Remove comments that state the obvious
From: Rob Herring @ 2016-10-27 12:18 UTC (permalink / raw)
To: Frank Rowand
Cc: Pantelis Antoniou, Pantelis Antoniou, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1477429146-27039-2-git-send-email-frowand.list@gmail.com>
On Tue, Oct 25, 2016 at 3:58 PM, <frowand.list@gmail.com> wrote:
> From: Frank Rowand <frank.rowand@am.sony.com>
>
> Remove comments that state the obvious, to reduce clutter
I'm probably not the best reviewer, have you ever seen a comment in my code. :)
>
> Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
> ---
> drivers/of/resolver.c | 31 ++-----------------------------
> 1 file changed, 2 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
> index 46325d6394cf..4ff0220d7aa2 100644
> --- a/drivers/of/resolver.c
> +++ b/drivers/of/resolver.c
> @@ -75,8 +73,6 @@ static phandle of_get_tree_max_phandle(void)
>
> /*
> * Adjust a subtree's phandle values by a given delta.
> - * Makes sure not to just adjust the device node's phandle value,
> - * but modify the phandle properties values as well.
What's missing here is the why? Why do we adjust phandle values?
> */
> static void __of_adjust_tree_phandles(struct device_node *node,
> int phandle_delta)
> @@ -85,32 +81,25 @@ static void __of_adjust_tree_phandles(struct device_node *node,
> struct property *prop;
> phandle phandle;
>
> - /* first adjust the node's phandle direct value */
Seems somewhat useful.
> if (node->phandle != 0 && node->phandle != OF_PHANDLE_ILLEGAL)
> node->phandle += phandle_delta;
>
> - /* now adjust phandle & linux,phandle values */
Seems somewhat useful.
> for_each_property_of_node(node, prop) {
>
> - /* only look for these two */
> if (of_prop_cmp(prop->name, "phandle") != 0 &&
> of_prop_cmp(prop->name, "linux,phandle") != 0)
> continue;
>
> - /* must be big enough */
> if (prop->length < 4)
> continue;
>
> - /* read phandle value */
> phandle = be32_to_cpup(prop->value);
> - if (phandle == OF_PHANDLE_ILLEGAL) /* unresolved */
> + if (phandle == OF_PHANDLE_ILLEGAL)
> continue;
>
> - /* adjust */
> *(uint32_t *)prop->value = cpu_to_be32(node->phandle);
> }
>
> - /* now do the children recursively */
> for_each_child_of_node(node, child)
> __of_adjust_tree_phandles(child, phandle_delta);
> }
> @@ -254,7 +239,6 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
>
> for (i = 0; i < count; i++) {
> off = be32_to_cpu(((__be32 *)rprop->value)[i]);
> - /* make sure the offset doesn't overstep (even wrap) */
Seems somewhat useful.
> if (off >= sprop->length ||
> (off + 4) > sprop->length) {
> pr_err("%s: Illegal property '%s' @%s\n",
> @@ -349,10 +328,8 @@ int of_resolve_phandles(struct device_node *resolve)
> resolve_sym = NULL;
> resolve_fix = NULL;
>
> - /* this may fail (if no fixups are required) */
Seem somewhat useful.
> root_sym = of_find_node_by_path("/__symbols__");
>
> - /* locate the symbols & fixups nodes on resolve */
> for_each_child_of_node(resolve, child) {
>
> if (!resolve_sym &&
^ permalink raw reply
* Re: [PATCH v2 3/4] arm64: arch_timer: Work around Erratum Hisilicon-161601
From: Ding Tianhong @ 2016-10-27 12:17 UTC (permalink / raw)
To: Marc Zyngier, catalin.marinas, will.deacon, mark.rutland, oss,
devicetree, shawnguo, stuart.yoder, linux-arm-kernel, linuxarm
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
* Re: [RFC PATCH 00/13] of: Make drivers/of/resolver.c more readable
From: Rob Herring @ 2016-10-27 12:03 UTC (permalink / raw)
To: Frank Rowand
Cc: Pantelis Antoniou, Pantelis Antoniou, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <580FC874.4000401@gmail.com>
On Tue, Oct 25, 2016 at 4:02 PM, Frank Rowand <frowand.list@gmail.com> wrote:
> On 10/25/16 13:58, frowand.list@gmail.com wrote:
>> From: Frank Rowand <frank.rowand@am.sony.com>
>>
>> drivers/of/resolve.c is a bit difficult to read. Clean it up so
>> that review of future overlay related patches will be easier.
>
> < snip >
>
> Hi Pantelis,
>
> Can you test this patch series on some real hardware?
Did you run unit tests?
Rob
^ permalink raw reply
* Re: [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: David Lechner, Kevin Hilman
Cc: Mark Rutland, devicetree, Russell King, linux-kernel, Rob Herring,
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
* Re: [PATCH 5/5] ARM: dts: Add LEGO MINDSTORTMS EV3 dts
From: Sekhar Nori @ 2016-10-27 11:18 UTC (permalink / raw)
To: David Lechner, Kevin Hilman
Cc: Mark Rutland, devicetree, Russell King, linux-kernel, Rob Herring,
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
* Re: [PATCH V3 2/2] iio: adc: spmi-vadc: Changes to support different scaling
From: Stanimir Varbanov @ 2016-10-27 11:18 UTC (permalink / raw)
To: Rama Krishna Phani A, linux-iio-u79uwXL29TY76Z2rM5mHXA,
jic23-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
smohanad-sgV2jX0FEOL9JmXXK+q4OQ, mgautam-sgV2jX0FEOL9JmXXK+q4OQ,
sivaa-sgV2jX0FEOL9JmXXK+q4OQ, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
Julia.Lawall-L2FTfq7BK8M
In-Reply-To: <1477492887-1663-3-git-send-email-rphani-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Hi Rama,
On 10/26/2016 05:41 PM, Rama Krishna Phani A wrote:
> Polling can also be used for End of conversion completion. Implement logic
> to choose either polling or interrupt for End of conversion completion.
> Scaling can be done on the voltage to report adc code in physical units.
> Add changes to support different scale functions to convert adc code to
> physical units.
>
> Signed-off-by: Rama Krishna Phani A <rphani-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> .../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt | 14 ++
> drivers/iio/adc/qcom-spmi-vadc.c | 263 +++++++++++++++++----
> 2 files changed, 236 insertions(+), 41 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
> index 0fb4613..39e31c0e 100644
> --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
> @@ -37,6 +37,12 @@ VADC node:
> Value type: <prop-encoded-array>
> Definition: End of conversion interrupt.
>
> +- qcom,vadc-poll-eoc:
> + Usage: optional
> + Value type: <bool>
> + Definition: Use polling instead of interrupts for End of Conversion
> + completion.
Why you need to add such a flag in DT?
The DT should describe hardware details not how the driver will choose
pooling vs interrupt.
On which use-case you would prefer pooling?
> +
> Channel node properties:
>
> - reg:
> @@ -92,6 +98,14 @@ Channel node properties:
> Valid values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512
> If property is not found, 1 sample will be used.
>
> +- qcom,scale-function:
> + Usage: optional
> + Value type: <u32>
> + Definition: Scaling function used to convert raw ADC code to
> + units specific to a given channel. Scaled units can be
> + microvolts, millidegC.Valid values are: 0, 1, 2, 3, 4.
> + If property is not found, 0 scaling will be used.
This shouldn't be in DT binding. Just select the scale function for each
channel in the driver based on compatible property.
--
regards,
Stan
^ permalink raw reply
* Re: [PATCH V2 3/4] regulator: pv88080: Update Regulator driver for MFD support
From: Mark Brown @ 2016-10-27 11:03 UTC (permalink / raw)
To: Eric Jeong
Cc: LINUX-KERNEL, Liam Girdwood, Alexandre Courbot, DEVICETREE,
LINUX-GPIO, Lee Jones, Linus Walleij, Mark Rutland, Rob Herring,
Support Opensource
In-Reply-To: <711afefe3e65c166111473024bf9b54f3d4cc8b6.1477530195.git.eric.jeong@diasemi.com>
[-- Attachment #1: Type: text/plain, Size: 990 bytes --]
On Thu, Oct 27, 2016 at 10:03:14AM +0900, Eric Jeong wrote:
> config REGULATOR_PV88080
> - tristate "Powerventure Semiconductor PV88080 regulator"
> - depends on I2C
> - select REGMAP_I2C
> + bool "Powerventure Semiconductor PV88080 regulator"
> + depends on MFD_PV88080
Forcing the driver to be built in looks like a regression, why would we
want to do that?
> + irq = platform_get_irq_byname(pdev, "regulator-irq");
> + if (irq < 0) {
> + dev_err(&pdev->dev, "Failed to get IRQ.\n");
> + return irq;
> }
What's the _byname() adding here given that the name is so generic? It
feels like if the name ever becomes important then this particular name
is going to be a problem.
> -module_i2c_driver(pv88080_regulator_driver);
> +static int __init pv88080_regulator_init(void)
> +{
> + return platform_driver_register(&pv88080_regulator_driver);
> +}
> +subsys_initcall(pv88080_regulator_init);
Why are you converting this to subsys_initcall()? This looks like
another regression.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ 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