* Re: [PATCH V5 2/7] ACPI / bus: Introduce acpi_get_match_data() function
From: Sinan Kaya @ 2017-12-04 22:01 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: dmaengine, Timur Tabi, devicetree@vger.kernel.org, linux-arm-msm,
linux-arm-kernel@lists.infradead.org, Rafael J. Wysocki,
Len Brown, open list:ACPI, open list
In-Reply-To: <CAJZ5v0gCkY80+A=cZKzhKhxKObSgqN3k1JGgGQgW5cwRSvqt_Q@mail.gmail.com>
On 12/4/2017 4:59 PM, Rafael J. Wysocki wrote:
> On Sat, Dec 2, 2017 at 5:27 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
>> OF has of_device_get_match_data() function to extract driver specific data
>> structure. Add a similar function for ACPI.
>>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>
> Haven't I ACKed this already?
>
> Anyway, please resend the whole series with a CC to linux-acpi.
Yeah, you acked the previous one. Then, I changed the calling parameter from
struct device to struct acpi_device and removed your ACK.
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH V5 1/7] Documentation: DT: qcom_hidma: Bump HW revision for the bugfixed HW
From: Rob Herring @ 2017-12-04 22:14 UTC (permalink / raw)
To: Sinan Kaya
Cc: dmaengine, timur, devicetree, linux-arm-msm, linux-arm-kernel,
Vinod Koul, Mark Rutland, open list
In-Reply-To: <1512188864-773-2-git-send-email-okaya@codeaurora.org>
On Fri, Dec 01, 2017 at 11:27:38PM -0500, Sinan Kaya wrote:
> A new version of the HIDMA IP has been released with bug fixes. Bumping the
> hardware version to differentiate from others.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
> index 55492c2..5d93d6d 100644
> --- a/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
> +++ b/Documentation/devicetree/bindings/dma/qcom_hidma_mgmt.txt
> @@ -47,8 +47,8 @@ When the OS is not in control of the management interface (i.e. it's a guest),
> the channel nodes appear on their own, not under a management node.
>
> Required properties:
> -- compatible: must contain "qcom,hidma-1.0" for initial HW or "qcom,hidma-1.1"
> -for MSI capable HW.
> +- compatible: must contain "qcom,hidma-1.0" for initial HW or
> + "qcom,hidma-1.1"/"qcom,hidma-1.2" for MSI capable HW.
Hopefully 1.2 corresponds to some actual version numbering and not just
something you made up. I'd really rather have SoC based compatible
strings unless you have dozens of SoCs for each version. I'll probably
just say the same thing again when 1.3 or 2.0 gets added...
Reviewed-by: Rob Herring <robh@kernel.org>
> - reg: Addresses for the transfer and event channel
> - interrupts: Should contain the event interrupt
> - desc-count: Number of asynchronous requests this channel can handle
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH v3 1/1] at24: support eeproms that do not auto-rollover reads.
From: Sven Van Asbroeck @ 2017-12-04 22:24 UTC (permalink / raw)
To: Sakari Ailus
Cc: Sven Van Asbroeck, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, wsa-z923LK4zBo2bacvFa/9K2g,
Bartosz Golaszewski, nsekhar-l0cyMroinI0,
david-nq/r/kbU++upp/zk7JDF2g, javier-0uQlZySMnqxg9hUCZPvPmw,
divagar.mohandass-ral2JQCrhuEAvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-i2c, Sven Van Asbroeck
In-Reply-To: <20171204214023.ml2ujacezsop5ilb-sGAanXTfQ4777SC2UrCW1FMQynFLKtET@public.gmane.org>
> If this is truly specific to at24, then vendor prefix would be appropriate,
> plus it'd go to an at24 specific binding file. However if it isn't I'd just
> remove the above sentence. I guess the latter?
Yes, no-read-rollover is truly specific to at24.c, because it applies only
to i2c multi-address chips. The at25 is spi based so cannot have multiple
addresses.
So yes, "at24,no-read-rollover" would perhaps be a better name.
Regarding an at24 specific binding file. You're saying I should create
Documentation/devicetree/bindings/eeprom/at24.txt ? Should I indicate
that at24.txt "inherits from" eeprom.txt? Note that at25.txt does not
currently do this.
--
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: [PATCH V5 4/7] OF: properties: Implement get_match_data() callback
From: Rob Herring @ 2017-12-04 22:25 UTC (permalink / raw)
To: Sinan Kaya
Cc: open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM, Timur Tabi,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-msm,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Frank Rowand, open list
In-Reply-To: <590f0536-b73d-ee32-a284-32e564af2f57-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On Mon, Dec 04, 2017 at 01:05:51PM -0500, Sinan Kaya wrote:
> On 12/4/2017 11:23 AM, Rob Herring wrote:
> > On Fri, Dec 1, 2017 at 10:27 PM, Sinan Kaya <okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> >> Now that we have a get_match_data() callback as part of the firmware node,
> >> implement the OF specific piece for it.
> >>
> >> Signed-off-by: Sinan Kaya <okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> >> ---
> >> drivers/of/property.c | 17 +++++++++++++++++
> >> 1 file changed, 17 insertions(+)
> >>
> ..
>
> >>
> >> +void *of_fwnode_get_match_data(const struct fwnode_handle *fwnode,
> >> + const struct device_driver *drv)
> >> +{
> >> + const struct device_node *node = to_of_node(fwnode);
> >> + const struct of_device_id *match;
> >> +
> >> + if (!node)
> >> + return NULL;
> >
> > of_match_node checks this.
>
> I see a check for the matches argument but not for the node argument.
> Am I missing something?
Ah yes, you are right.
>
> >
> >> +
> >> + match = of_match_node(drv->of_match_table, node);
> >> + if (!match)
> >> + return NULL;
> >> +
> >> + return (void *)match->data;
> >
> > Don't need a cast here.
>
> I can fix this.
>
> >
> > of_device_get_match_data() already does most of this, but getting a
> > device ptr from fwnode_handle may not be possible?
>
> I couldn't figure out how to do that. Do you have a suggestion?
> I have been looking for examples with no luck.
Change the property API to pass struct device instead. That's maybe not
worth it.
Rob
--
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: [PATCH net-next 1/2 v6] net: ethernet: Add DT bindings for the Gemini ethernet
From: Rob Herring @ 2017-12-04 22:30 UTC (permalink / raw)
To: Linus Walleij
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S . Miller,
Michał Mirosław, Janos Laube, Paulius Zaleckas,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Hans Ulli Kroll, Florian Fainelli,
devicetree-u79uwXL29TY76Z2rM5mHXA, Tobias Waldvogel
In-Reply-To: <20171202110640.5284-1-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Sat, Dec 02, 2017 at 12:06:39PM +0100, Linus Walleij wrote:
> This adds the device tree bindings for the Gemini ethernet
> controller. It is pretty straight-forward, using standard
> bindings and modelling the two child ports as child devices
> under the parent ethernet controller device.
>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Tobias Waldvogel <tobias.waldvogel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> .../bindings/net/cortina,gemini-ethernet.txt | 92 ++++++++++++++++++++++
> 1 file changed, 92 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/cortina,gemini-ethernet.txt
>
> diff --git a/Documentation/devicetree/bindings/net/cortina,gemini-ethernet.txt b/Documentation/devicetree/bindings/net/cortina,gemini-ethernet.txt
> new file mode 100644
> index 000000000000..35fa3abd1c73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/cortina,gemini-ethernet.txt
> @@ -0,0 +1,92 @@
> +Cortina Systems Gemini Ethernet Controller
> +==========================================
> +
> +This ethernet controller is found in the Gemini SoC family:
> +StorLink SL3512 and SL3516, also known as Cortina Systems
> +CS3512 and CS3516.
> +
> +Required properties:
> +- compatible: must be "cortina,gemini-ethernet"
> +- reg: must contain the global registers and the V-bit and A-bit
> + memory areas, in total three register sets.
> +- syscon: a phandle to the system controller
> +- #address-cells: must be specified, must be <1>
> +- #size-cells: must be specified, must be <1>
> +- ranges: should be state like this giving a 1:1 address translation
> + for the subnodes
> +
> +The subnodes represents the two ethernet ports in this device.
> +They are not independent of each other since they share resources
> +in the parent node, and are thus children.
> +
> +Required subnodes:
> +- port0: contains the resources for ethernet port 0
> +- port1: contains the resources for ethernet port 1
> +
> +Required subnode properties:
> +- compatible: must be "cortina,gemini-ethernet-port"
> +- reg: must contain two register areas: the DMA/TOE memory and
> + the GMAC memory area of the port
> +- interrupts: should contain the interrupt line of the port.
> + this is nominally a level interrupt active high.
> +- resets: this must provide an SoC-integrated reset line for
> + the port.
> +- clocks: this should contain a handle to the PCLK clock for
> + clocking the silicon in this port
> +- clock-names: must be "PCLK"
> +
> +Optional subnode properties:
> +- phy-mode: see ethernet.txt
> +- phy-handle: see ethernet.txt
> +
> +Example:
> +
> +mdio-bus {
> + (...)
> + phy0: ethernet-phy@1 {
> + reg = <1>;
> + device_type = "ethernet-phy";
> + };
> + phy1: ethernet-phy@3 {
> + reg = <3>;
> + device_type = "ethernet-phy";
> + };
> +};
> +
> +
> +ethernet@60000000 {
> + compatible = "cortina,gemini-ethernet";
> + reg = <0x60000000 0x4000>, /* Global registers, queue */
> + <0x60004000 0x2000>, /* V-bit */
> + <0x60006000 0x2000>; /* A-bit */
> + syscon = <&syscon>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
Would be better to define the actual range used by child nodes.
> +
> + gmac0: port0 {
Needs a unit-address. Building with W=1 (or W=2) will tell you this.
As port is used by the OF graph binding, use ethernet-port@... instead.
> + compatible = "cortina,gemini-ethernet-port";
> + reg = <0x60008000 0x2000>, /* Port 0 DMA/TOE */
> + <0x6000a000 0x2000>; /* Port 0 GMAC */
> + interrupt-parent = <&intcon>;
> + interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
> + resets = <&syscon GEMINI_RESET_GMAC0>;
> + clocks = <&syscon GEMINI_CLK_GATE_GMAC0>;
> + clock-names = "PCLK";
> + phy-mode = "rgmii";
> + phy-handle = <&phy0>;
> + };
> +
> + gmac1: port1 {
> + compatible = "cortina,gemini-ethernet-port";
> + reg = <0x6000c000 0x2000>, /* Port 1 DMA/TOE */
> + <0x6000e000 0x2000>; /* Port 1 GMAC */
> + interrupt-parent = <&intcon>;
> + interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
> + resets = <&syscon GEMINI_RESET_GMAC1>;
> + clocks = <&syscon GEMINI_CLK_GATE_GMAC1>;
> + clock-names = "PCLK";
> + phy-mode = "rgmii";
> + phy-handle = <&phy1>;
> + };
> +};
> --
> 2.14.3
>
> --
> 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: [PATCH] pinctrl: gemini: Support drive strength setting
From: Rob Herring @ 2017-12-04 22:32 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171202112309.5726-1-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Sat, Dec 02, 2017 at 12:23:09PM +0100, Linus Walleij wrote:
> The Gemini pin controller can set drive strength for a few
> select groups of pins (not individually). Implement this
> for GMAC0 and 1 (ethernet ports), IDE and PCI.
>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> The DT binding part is just using generic bindings, so should be
> pretty uncontroversial.
> ---
> .../bindings/pinctrl/cortina,gemini-pinctrl.txt | 3 +
> drivers/pinctrl/pinctrl-gemini.c | 81 ++++++++++++++++++++++
> 2 files changed, 84 insertions(+)
Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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: [PATCH v2 1/4] dt-bindings: thermal/armada: describe AP806 and CP110
From: Rob Herring @ 2017-12-04 22:33 UTC (permalink / raw)
To: Baruch Siach
Cc: Zhang Rui, Eduardo Valentin, devicetree-u79uwXL29TY76Z2rM5mHXA,
Jason Cooper, Andrew Lunn, Gregory Clement, Sebastian Hesselbarth,
Russell King, Miquel Raynal, linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <f8c589337a4fb78852eadf15058e8f8d132d4dc0.1512299484.git.baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
On Sun, Dec 03, 2017 at 01:11:21PM +0200, Baruch Siach wrote:
> Add compatible strings for AP806 and CP110 that are part of the Armada 8k/7k
> line of SoCs.
>
> Add a note on the difference in the size of the control area in different
> bindings. This is an existing difference between the Armada 375 binding and
> the rest. The new AP806 and CP110 bindings are similar to the existing Armada
> 375 in this regard.
>
> Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
> ---
> v2: No change
> ---
> Documentation/devicetree/bindings/thermal/armada-thermal.txt | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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: [PATCH v6 4/6] dt: bindings: lp8860: Update the bindings to the standard
From: Rob Herring @ 2017-12-04 22:35 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Dan Murphy, rpurdie, pavel, linux-leds, linux-kernel,
devicetree@vger.kernel.org
In-Reply-To: <a07f883c-4c4c-780d-2dac-f023907a2629@gmail.com>
On Sun, Dec 03, 2017 at 02:27:20PM +0100, Jacek Anaszewski wrote:
> Dan,
>
> On 12/01/2017 05:56 PM, Dan Murphy wrote:
> > Update the lp8860 dt binding to the LED standard where
> > the LED should have a child node and also adding a
> > LED trigger entry.
> >
> > Signed-off-by: Dan Murphy <dmurphy@ti.com>
> > ---
> >
> > v6 - New patch to fix binding documentation
> >
> > Documentation/devicetree/bindings/leds/leds-lp8860.txt | 15 +++++++++++----
> > 1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
> > index aad38dd94d4b..4cf396de6eba 100644
> > --- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt
> > +++ b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
> > @@ -12,17 +12,24 @@ Required properties:
> > - label - Used for naming LEDs
> >
> > Optional properties:
> > - - enable-gpio - gpio pin to enable/disable the device.
> > - - supply - "vled" - LED supply
> > + - enable-gpios : gpio pin to enable/disable the device.
> > + - vled-supply : LED supply
> > + - label : see Documentation/devicetree/bindings/leds/common.txt
> > + - linux,default-trigger : (optional)
> > + see Documentation/devicetree/bindings/leds/common.txt
> >
> > Example:
> >
> > -leds: leds@6 {
> > +lp8860@2d {
leds@2d
> > compatible = "ti,lp8860";
> > reg = <0x2d>;
> > - label = "display_cluster";
> > enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>;
> > vled-supply = <&vbatt>;
> > +
> > + backlight: backlight@0 {
unit-address requires a 'reg' property. Building your dts files with W=1
will tell you this.
> > + label = "backlight_cluster";
>
> You'll need to change this to:
>
> label = "white:backlight_cluster"
>
> Please always cc your patches with DT bindings to
> devicetree@vger.kernel.org and related maintainers.
>
> > + linux,default-trigger = "backlight";
> > + };
> > }
> >
> > For more product information please see the link below:
> >
>
> --
> Best regards,
> Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH] dt-bindings: mtd: fsl-quadspi: Pass the qspi clock names
From: Rob Herring @ 2017-12-04 22:36 UTC (permalink / raw)
To: Fabio Estevam
Cc: boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
cyrille.pitchen-yU5RGvR974pGWvitb5QawA,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Fabio Estevam
In-Reply-To: <1512340584-26894-1-git-send-email-festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Sun, Dec 03, 2017 at 08:36:24PM -0200, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
>
> In order to improve the bindings documentation, explicitly pass the name
> of the clocks: "qspi_en" and "qspi", which are mandatory.
>
> Signed-off-by: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
> ---
> Documentation/devicetree/bindings/mtd/fsl-quadspi.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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: [PATCH 1/3] dt-bindings: pwm: update bindings for the Meson-AXG
From: Rob Herring @ 2017-12-04 22:39 UTC (permalink / raw)
To: Yixun Lan
Cc: Thierry Reding, Kevin Hilman, linux-pwm, linux-amlogic,
devicetree, Neil Armstrong, Jerome Brunet, Mark Rutland,
Carlo Caione, Jian Hu, linux-arm-kernel, linux-kernel
In-Reply-To: <20171204060018.8856-2-yixun.lan@amlogic.com>
On Mon, Dec 04, 2017 at 02:00:16PM +0800, Yixun Lan wrote:
> From: Jian Hu <jian.hu@amlogic.com>
>
> Update the doc to explicitly support Meson-AXG
>
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> ---
> Documentation/devicetree/bindings/pwm/pwm-meson.txt | 2 ++
> 1 file changed, 2 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 3/4] RFC: net: dsa: Add bindings for Realtek SMI DSAs
From: Andrew Lunn @ 2017-12-04 22:50 UTC (permalink / raw)
To: Linus Walleij
Cc: Florian Fainelli, Vivien Didelot, netdev-u79uwXL29TY76Z2rM5mHXA,
Antti Seppälä, Roman Yeryomin, Colin Leitner,
Gabor Juhos,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <CACRpkdYoMVNh8eaTnaDQ59bsh4bC88biLaYSXyhnc4W83PMWzA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
> So why not:
>
> switch@0 {
> compatible = "acme,switch";
> #address-cells = <1>;
> #size-cells = <0>;
>
> ports {
>
> port@0 {
> reg = <0>;
> phy@0 {
> reg = <0>;
> };
> };
Hi Linus
So you are suggesting put the PHY node inside the MAC node.
This is sometimes done, but does not describe the hardware. The PHYs
are on an MDIO bus, so device tree should show the MDIO bus and the
PHYs on it.
DSA does have an MDIO bus, so putting the PHYs in the MAC is just
confusing. Although that is not Florians preferred solution, he would
like the DSA driver to export its own MDIO bus and list the PHYs on
it.
Andrew
--
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: [PATCH v6 1/2] media: ov7740: Document device tree bindings
From: Yang, Wenyou @ 2017-12-05 0:48 UTC (permalink / raw)
To: Rob Herring
Cc: Mauro Carvalho Chehab, Mark Rutland, linux-kernel, Nicolas Ferre,
devicetree, Sakari Ailus, Jonathan Corbet, Hans Verkuil,
linux-arm-kernel, Linux Media Mailing List
In-Reply-To: <20171204202443.jiizsqu6yfpsugj4@rob-hp-laptop>
[-- Attachment #1: Type: text/plain, Size: 756 bytes --]
Hi Rob,
On 2017/12/5 4:24, Rob Herring wrote:
> On Mon, Dec 04, 2017 at 02:58:57PM +0800, Wenyou Yang wrote:
>> Add the device tree binding documentation for the ov7740 sensor driver.
>>
>> Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
>> ---
>>
>> Changes in v6: None
>> Changes in v5: None
>> Changes in v4: None
>> Changes in v3:
>> - Explicitly document the "remote-endpoint" property.
>>
>> Changes in v2: None
>>
>> .../devicetree/bindings/media/i2c/ov7740.txt | 47 ++++++++++++++++++++++
>> 1 file changed, 47 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7740.txt
> Please add acks when posting new versions.
Sorry, I forgot it.
Will add it in next version.
Best Regards,
Wenyou Yang
[-- Attachment #2: Type: text/html, Size: 1380 bytes --]
^ permalink raw reply
* Re: [PATCH 4/8] ASoC: uniphier: add support for UniPhier EVEA codec
From: Masahiro Yamada @ 2017-12-05 0:58 UTC (permalink / raw)
To: Mark Brown
Cc: Katsuhiro Suzuki, alsa-devel, Rob Herring, devicetree,
Masami Hiramatsu, Jassi Brar, linux-arm-kernel,
Linux Kernel Mailing List, Thomas Gleixner, Greg Kroah-Hartman
In-Reply-To: <20171204182027.2k5uw5og65h6ac7w@sirena.org.uk>
+CC Greg-KH
+CC Thomas Gleixner
2017-12-05 3:20 GMT+09:00 Mark Brown <broonie@kernel.org>:
> On Wed, Nov 22, 2017 at 08:43:17PM +0900, Katsuhiro Suzuki wrote:
>
>> +++ b/sound/soc/uniphier/evea.c
>> @@ -0,0 +1,567 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Socionext UniPhier EVEA ADC/DAC codec driver.
>> + *
>> + * Copyright (c) 2016-2017 Socionext Inc.
>
> Make the entire comment a C++ comment, don't mix and match like this -
> it's ugly. Otherwise this looks good so I'll apply it, please send a
> followup patch fixing this (and I guess the same thing will apply to
> other files).
Indeed ugly,
but I think this is intentional to make the SPDX line stand out.
Linus suggested this as far as I understood from the following:
https://patchwork.kernel.org/patch/10016201/
If you use C++ comment style for the entire block,
it will not stand out.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH 0/2] of: dynamic: restrict overlay by targets
From: Frank Rowand @ 2017-12-05 1:14 UTC (permalink / raw)
To: Alan Tull, Moritz Fischer, Rob Herring, Pantelis Antoniou
Cc: devicetree, linux-kernel, linux-fpga
In-Reply-To: <20171204191357.3211-1-atull@kernel.org>
Hi Alan,
In the RFC thread "of: Add whitelist", I did not understand the use case and
asked you some questions (30 Nov 2017 07:46:36 -0500), that you seem to have
overlooked (or my mail server failed to deliver your answer to me). Can you
please answer that question so I can better understand this patch set is
needed for.
Thanks,
Frank
On 12/04/17 14:13, Alan Tull wrote:
> Restrict which nodes are valid targets for a DT overlay.
>
> Add a flag bit to struct device_node allowing nodes to be marked as
> valid target for overlays.
>
> A driver that is always intended to handle DT overlays can
> enable overlays by calling a function for its DT node.
>
> For individual nodes that need to be opened up for a specific use,
> adding the property "overlay-allowed" enables overlays targeting
> that node. I'll need to document the DT property, not sure where
> specifically. New file bindings/overlay.txt?
>
> This patchset differs from the RFC:
> * Added a flag bit and got rid of the whitelist
> * Renamed the functions that enable a node
> * Added a DT property
>
> Alan Tull (2):
> of: overlay: add flag enabling overlays and enable fpga-region
> overlays
> of: dynamic: add overlay-allowed DT property
>
> drivers/fpga/of-fpga-region.c | 4 ++++
> drivers/of/base.c | 4 ++--
> drivers/of/dynamic.c | 3 +++
> drivers/of/fdt.c | 3 +++
> drivers/of/of_private.h | 2 ++
> drivers/of/overlay.c | 26 ++++++++++++++++++++++++++
> include/linux/of.h | 19 +++++++++++++++++++
> 7 files changed, 59 insertions(+), 2 deletions(-)
>
^ permalink raw reply
* Re: [PATCH v2 2/2] of: overlay: Fix cleanup order in of_overlay_apply()
From: Frank Rowand @ 2017-12-05 1:16 UTC (permalink / raw)
To: Geert Uytterhoeven, Rob Herring
Cc: Geert Uytterhoeven, Pantelis Antoniou, Colin King, Dan Carpenter,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdVFR3FPVAM2J_L2HvNUzkJsQM57SJJ2CMux71M4dhZYfQ@mail.gmail.com>
On 12/04/17 14:45, Geert Uytterhoeven wrote:
> Hi Rob,
>
> On Mon, Dec 4, 2017 at 8:35 PM, Rob Herring <robh+dt@kernel.org> wrote:
>> On Mon, Dec 4, 2017 at 9:47 AM, Geert Uytterhoeven
>> <geert+renesas@glider.be> wrote:
>>> The special overlay mutex is taken first, hence it should be released
>>> last in the error path.
>>>
>>> Move "mutex_lock(&of_mutex)" up, as suggested by Frank, as
>>> free_overlay_changeset() should be called with that mutex held if any
>>> non-trivial cleanup is to be done.
>>
>> Not holding the of_mutex for of_resolve_phandles is just wrong.
>> Without it, a node and new phandle could be added via of_attach_node
>> making the max phandle wrong.
>
> After my patch it's held, so what's the problem?
>
>> Now, with the 2 mutexes adjacent, what is the point of even having the
>> of_overlay_mutex? Seems like we should just drop it.
>
> Frank?
__of_changeset_apply_notify(), which is called by __of_changeset_apply()
unlocks of_mutex, then does notifications then locks of_mutex. So the
mutex get released in the middle of of_overlay_apply()
I have never been comfortable with the unlock/lock there, but don't have
an alternative yet.
>> I also don't think we really need to hold the mutex during post-apply
>> notifiers. It also seems like some steps could be moved outside the
>> mutex(es) like init_overlay_changeset().
>
> Perhaps.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
^ permalink raw reply
* Re: [PATCH net-next v4 2/2] net: ethernet: socionext: add AVE ethernet driver
From: Kunihiko Hayashi @ 2017-12-05 1:42 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, Florian Fainelli, Rob Herring, Mark Rutland,
linux-arm-kernel, linux-kernel, devicetree, Masahiro Yamada,
Masami Hiramatsu, Jassi Brar
In-Reply-To: <20171201134900.GE22599@lunn.ch>
Hi Andrew,
On Fri, 1 Dec 2017 14:49:00 +0100 Andrew Lunn <andrew@lunn.ch> wrote:
> On Fri, Dec 01, 2017 at 10:03:50AM +0900, Kunihiko Hayashi wrote:
> > The UniPhier platform from Socionext provides the AVE ethernet
> > controller that includes MAC and MDIO bus supporting RGMII/RMII
> > modes. The controller is named AVE.
> >
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>
> Hi Kunihiko
>
> I reviewed the PHY and MDIO code. It all looks good. I cannot say much
> about the rest of it though.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Thanks! I'll remove warning from kbuild test, and repost the series for
reviewing other part.
---
Best Regards,
Kunihiko Hayashi
^ permalink raw reply
* Re: [PATCH V1 1/4] qcom: spmi-wled: Add support for qcom wled driver
From: Bjorn Andersson @ 2017-12-05 2:01 UTC (permalink / raw)
To: Kiran Gunda
Cc: linux-arm-msm, Lee Jones, Daniel Thompson, Jingoo Han,
Richard Purdie, Jacek Anaszewski, Pavel Machek, Rob Herring,
Mark Rutland, Bartlomiej Zolnierkiewicz, linux-leds, devicetree,
linux-kernel, linux-fbdev, linux-arm-msm-owner
In-Reply-To: <1510834717-21765-2-git-send-email-kgunda@codeaurora.org>
On Thu 16 Nov 04:18 PST 2017, Kiran Gunda wrote:
> WLED driver provides the interface to the display driver to
> adjust the brightness of the display backlight.
>
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> ---
> .../bindings/leds/backlight/qcom-spmi-wled.txt | 90 ++++
> drivers/video/backlight/Kconfig | 9 +
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/qcom-spmi-wled.c | 504 +++++++++++++++++++++
> 4 files changed, 604 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
> create mode 100644 drivers/video/backlight/qcom-spmi-wled.c
>
> diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
> new file mode 100644
> index 0000000..f1ea25b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
> @@ -0,0 +1,90 @@
> +Binding for Qualcomm WLED driver
> +
This binding document quite well describe the pm8941 as well, so please
improve the existing binding (changing to this style is preferable).
> +WLED (White Light Emitting Diode) driver is used for controlling display
> +backlight that is part of PMIC on Qualcomm Technologies reference platforms.
> +The PMIC is connected to the host processor via SPMI bus.
> +
> +- compatible
> + Usage: required
> + Value type: <string>
> + Definition: should be "qcom,pm8998-spmi-wled".
There's no WLED in the pm8998, so please make this pmi8998. This pmic is
SPMI only, so there's no need to keep "spmi" in the compatible.
> +
> +- reg
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: Base address and size of the WLED modules.
> +
> +- reg-names
> + Usage: required
> + Value type: <string>
> + Definition: Names associated with base addresses. should be
> + "qcom-wled-ctrl-base", "qcom-wled-sink-base".
> +
> +- label
> + Usage: required
> + Value type: <string>
> + Definition: The name of the backlight device.
> +
> +- default-brightness
> + Usage: optional
> + Value type: <u32>
> + Definition: brightness value on boot, value from: 0-4095
> + default: 2048
> +
> +- qcom,fs-current-limit
> + Usage: optional
> + Value type: <u32>
> + Definition: per-string full scale current limit in uA. value from
> + 0 to 30000 with 5000 uA resolution. default: 25000 uA
"in steps of 5mA"
> +
> +- qcom,current-boost-limit
> + Usage: optional
> + Value type: <u32>
> + Definition: ILIM threshold in mA. values are 105, 280, 450, 620, 970,
> + 1150, 1300, 1500. default: 970 mA
> +
> +- qcom,switching-freq
> + Usage: optional
> + Value type: <u32>
> + Definition: Switching frequency in KHz. values are
> + 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371,
> + 1600, 1920, 2400, 3200, 4800, 9600.
> + default: 800 KHz
> +
> +- qcom,ovp
> + Usage: optional
> + Value type: <u32>
> + Definition: Over-voltage protection limit in mV. values are 31100,
> + 29600, 19600, 18100.
> + default: 29600 mV
> +
> +- qcom,string-cfg
> + Usage: optional
> + Value type: <u32>
> + Definition: Bit mask of the wled strings. Bit 0 to 3 indicates strings
> + 0 to 3 respectively. Wled module has four strings of leds
> + numbered from 0 to 3. Each string of leds are operated
> + individually. Specify the strings using the bit mask. Any
> + combination of led strings can be used.
> + default value is 15 (b1111).
Please try to avoid expressing things as bitmasks in DT.
The only difference from 8941 here is that there's one additional
string, so please start off by expressing this as the existing binding.
If you really need this flexibility you can follow up with an addition
of a property like this, but name it something like
"qcom,enabled-strings" and make this support available for pm8941 as
well.
> +
> +- qcom,en-cabc
No need for the "en", the presence of a bool property means that it's
enabled.
> + Usage: optional
> + Value type: <bool>
> + Definition: Specify if cabc (content adaptive backlight control) is
> + needed.
I presume cabc isn't ever "needed", just make the description "Enable
content adaptive backlight control".
> +
> +Example:
> +
> +qcom-wled@d800 {
> + compatible = "qcom,pm8998-spmi-wled";
> + reg = <0xd800 0xd900>;
> + reg-names = "qcom-wled-ctrl-base", "qcom-wled-sink-base";
> + label = "backlight";
> +
> + qcom,fs-current-limit = <25000>;
> + qcom,current-boost-limit = <970>;
> + qcom,switching-freq = <800>;
> + qcom,ovp = <29600>;
> + qcom,string-cfg = <15>;
> +};
[..]
> diff --git a/drivers/video/backlight/qcom-spmi-wled.c b/drivers/video/backlight/qcom-spmi-wled.c
After reviewing your arguments and comparing the drivers I still think
it's beneficial to support both these hardware revisions in the same
driver.
The majority of the register differences relates to the current sink
being split out, but this can easily be handled by a few well places
accessor functions - which depends on this being the case or not.
The addition of OVP handling would benefit 8941 as well.
The short circuit handling in your patches are isolated and not taking
this code path on 8941 should not pose any problems.
[..]
> +/* General definitions */
> +#define QCOM_WLED_DEFAULT_BRIGHTNESS 2048
> +#define QCOM_WLED_MAX_BRIGHTNESS 4095
> +
> +/* WLED control registers */
> +#define QCOM_WLED_CTRL_MOD_ENABLE 0x46
> +#define QCOM_WLED_CTRL_MOD_EN_MASK BIT(7)
> +#define QCOM_WLED_CTRL_MODULE_EN_SHIFT 7
> +
> +#define QCOM_WLED_CTRL_SWITCH_FREQ 0x4c
> +#define QCOM_WLED_CTRL_SWITCH_FREQ_MASK GENMASK(3, 0)
> +
> +#define QCOM_WLED_CTRL_OVP 0x4d
> +#define QCOM_WLED_CTRL_OVP_MASK GENMASK(1, 0)
> +
> +#define QCOM_WLED_CTRL_ILIM 0x4e
> +#define QCOM_WLED_CTRL_ILIM_MASK GENMASK(2, 0)
> +
> +/* WLED sink registers */
> +#define QCOM_WLED_SINK_CURR_SINK_EN 0x46
> +#define QCOM_WLED_SINK_CURR_SINK_MASK GENMASK(7, 4)
> +#define QCOM_WLED_SINK_CURR_SINK_SHFT 0x04
Shifts are typically not given as hex...
> +
> +#define QCOM_WLED_SINK_SYNC 0x47
> +#define QCOM_WLED_SINK_SYNC_MASK GENMASK(3, 0)
> +#define QCOM_WLED_SINK_SYNC_LED1 BIT(0)
> +#define QCOM_WLED_SINK_SYNC_LED2 BIT(1)
> +#define QCOM_WLED_SINK_SYNC_LED3 BIT(2)
> +#define QCOM_WLED_SINK_SYNC_LED4 BIT(3)
> +#define QCOM_WLED_SINK_SYNC_CLEAR 0x00
> +
> +#define QCOM_WLED_SINK_MOD_EN_REG(n) (0x50 + (n * 0x10))
> +#define QCOM_WLED_SINK_REG_STR_MOD_MASK BIT(7)
> +#define QCOM_WLED_SINK_REG_STR_MOD_EN BIT(7)
> +
> +#define QCOM_WLED_SINK_SYNC_DLY_REG(n) (0x51 + (n * 0x10))
> +#define QCOM_WLED_SINK_FS_CURR_REG(n) (0x52 + (n * 0x10))
> +#define QCOM_WLED_SINK_FS_MASK GENMASK(3, 0)
> +
> +#define QCOM_WLED_SINK_CABC_REG(n) (0x56 + (n * 0x10))
> +#define QCOM_WLED_SINK_CABC_MASK BIT(7)
> +#define QCOM_WLED_SINK_CABC_EN BIT(7)
> +
> +#define QCOM_WLED_SINK_BRIGHT_LSB_REG(n) (0x57 + (n * 0x10))
> +#define QCOM_WLED_SINK_BRIGHT_MSB_REG(n) (0x58 + (n * 0x10))
> +
> +struct qcom_wled_config {
> + u32 i_boost_limit;
> + u32 ovp;
> + u32 switch_freq;
> + u32 fs_current;
> + u32 string_cfg;
> + bool en_cabc;
> +};
> +
> +struct qcom_wled {
> + const char *name;
> + struct platform_device *pdev;
Lug around the struct device * instead of the platform_device, and use
this for dev_* prints throughout the code.
> + struct regmap *regmap;
> + u16 sink_addr;
> + u16 ctrl_addr;
> + u32 brightness;
> + bool prev_state;
You can derive prev_state from wled->brightness in
qcom_wled_update_status().
> +
> + struct qcom_wled_config cfg;
> +};
> +
> +static int qcom_wled_module_enable(struct qcom_wled *wled, int val)
> +{
> + int rc;
> +
> + rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
> + QCOM_WLED_CTRL_MOD_ENABLE, QCOM_WLED_CTRL_MOD_EN_MASK,
> + val << QCOM_WLED_CTRL_MODULE_EN_SHIFT);
This shift obfuscate the fact that val is only 0 or 1, make val a bool
and make the macro for the enabled state be BIT(7).
> + return rc;
> +}
> +
> +static int qcom_wled_get_brightness(struct backlight_device *bl)
> +{
> + struct qcom_wled *wled = bl_get_data(bl);
> +
> + return wled->brightness;
> +}
> +
> +static int qcom_wled_sync_toggle(struct qcom_wled *wled)
> +{
> + int rc;
> +
> + rc = regmap_update_bits(wled->regmap,
> + wled->sink_addr + QCOM_WLED_SINK_SYNC,
> + QCOM_WLED_SINK_SYNC_MASK, QCOM_WLED_SINK_SYNC_MASK);
> + if (rc < 0)
> + return rc;
> +
> + rc = regmap_update_bits(wled->regmap,
> + wled->sink_addr + QCOM_WLED_SINK_SYNC,
> + QCOM_WLED_SINK_SYNC_MASK, QCOM_WLED_SINK_SYNC_CLEAR);
> +
> + return rc;
> +}
> +
> +static int qcom_wled_set_brightness(struct qcom_wled *wled, u16 brightness)
> +{
> + int rc, i;
> + u16 low_limit = QCOM_WLED_MAX_BRIGHTNESS * 4 / 1000;
> + u8 string_cfg = wled->cfg.string_cfg;
> + u8 v[2];
> +
> + /* WLED's lower limit of operation is 0.4% */
> + if (brightness > 0 && brightness < low_limit)
> + brightness = low_limit;
What happens between 0 and 0.4%? Is this policy or is this related to
some hardware issue?
Also, this function will not be called with brightness = 0, so you don't
need to check that case.
> +
> + v[0] = brightness & 0xff;
> + v[1] = (brightness >> 8) & 0xf;
> +
> + for (i = 0; (string_cfg >> i) != 0; i++) {
The condition looks optimal... Just loop from 0 to 3 and it will be
easier to read without any measurable losses.
> + if (string_cfg & BIT(i)) {
Flip this condition around and use "continue" to reduce the indentation
level of the rest of the block.
> + rc = regmap_bulk_write(wled->regmap, wled->sink_addr +
> + QCOM_WLED_SINK_BRIGHT_LSB_REG(i), v, 2);
> + if (rc < 0)
> + return rc;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int qcom_wled_update_status(struct backlight_device *bl)
> +{
> + struct qcom_wled *wled = bl_get_data(bl);
> + u16 brightness = bl->props.brightness;
> + int rc;
> +
> + if (bl->props.power != FB_BLANK_UNBLANK ||
> + bl->props.fb_blank != FB_BLANK_UNBLANK ||
> + bl->props.state & BL_CORE_FBBLANK)
> + brightness = 0;
> +
> + if (brightness) {
> + rc = qcom_wled_set_brightness(wled, brightness);
> + if (rc < 0) {
> + pr_err("wled failed to set brightness rc:%d\n", rc);
Use dev_err() and dev_dbg() throughout the driver.
> + return rc;
> + }
> +
> + if (!!brightness != wled->prev_state) {
> + rc = qcom_wled_module_enable(wled, !!brightness);
> + if (rc < 0) {
> + pr_err("wled enable failed rc:%d\n", rc);
> + return rc;
> + }
> + }
This block is exactly the same as the else statement, there's no need to
repeat yourself.
> + } else {
> + rc = qcom_wled_module_enable(wled, brightness);
> + if (rc < 0) {
> + pr_err("wled disable failed rc:%d\n", rc);
> + return rc;
> + }
> + }
> +
> + wled->prev_state = !!brightness;
> +
> + rc = qcom_wled_sync_toggle(wled);
> + if (rc < 0) {
> + pr_err("wled sync failed rc:%d\n", rc);
> + return rc;
> + }
> +
> + wled->brightness = brightness;
> +
> + return rc;
> +}
> +
> +static int qcom_wled_setup(struct qcom_wled *wled)
> +{
> + int rc, temp, i;
> + u8 sink_en = 0;
> + u8 string_cfg = wled->cfg.string_cfg;
> +
> + rc = regmap_update_bits(wled->regmap,
> + wled->ctrl_addr + QCOM_WLED_CTRL_OVP,
> + QCOM_WLED_CTRL_OVP_MASK, wled->cfg.ovp);
> + if (rc < 0)
> + return rc;
> +
> + rc = regmap_update_bits(wled->regmap,
> + wled->ctrl_addr + QCOM_WLED_CTRL_ILIM,
> + QCOM_WLED_CTRL_ILIM_MASK, wled->cfg.i_boost_limit);
> + if (rc < 0)
> + return rc;
> +
> + rc = regmap_update_bits(wled->regmap,
> + wled->ctrl_addr + QCOM_WLED_CTRL_SWITCH_FREQ,
> + QCOM_WLED_CTRL_SWITCH_FREQ_MASK, wled->cfg.switch_freq);
> + if (rc < 0)
> + return rc;
> +
> + for (i = 0; (string_cfg >> i) != 0; i++) {
> + if (string_cfg & BIT(i)) {
Same as above.
> + u16 addr = wled->sink_addr +
> + QCOM_WLED_SINK_MOD_EN_REG(i);
> +
> + rc = regmap_update_bits(wled->regmap, addr,
> + QCOM_WLED_SINK_REG_STR_MOD_MASK,
> + QCOM_WLED_SINK_REG_STR_MOD_EN);
> + if (rc < 0)
> + return rc;
> +
> + addr = wled->sink_addr +
> + QCOM_WLED_SINK_FS_CURR_REG(i);
> + rc = regmap_update_bits(wled->regmap, addr,
> + QCOM_WLED_SINK_FS_MASK,
> + wled->cfg.fs_current);
> + if (rc < 0)
> + return rc;
> +
> + addr = wled->sink_addr +
> + QCOM_WLED_SINK_CABC_REG(i);
> + rc = regmap_update_bits(wled->regmap, addr,
> + QCOM_WLED_SINK_CABC_MASK,
> + wled->cfg.en_cabc ?
> + QCOM_WLED_SINK_CABC_EN : 0);
> + if (rc)
> + return rc;
> +
> + temp = i + QCOM_WLED_SINK_CURR_SINK_SHFT;
> + sink_en |= 1 << temp;
I'm failing to see the reason for the "temp" variable here. Please do:
sink_en |= BIT(i + QCOM_WLED_SINK_CURR_SINK_SHFT)
> + }
> + }
> +
> + rc = regmap_update_bits(wled->regmap,
> + wled->sink_addr + QCOM_WLED_SINK_CURR_SINK_EN,
> + QCOM_WLED_SINK_CURR_SINK_MASK, sink_en);
> + if (rc < 0)
> + return rc;
> +
> + rc = qcom_wled_sync_toggle(wled);
> + if (rc < 0) {
> + pr_err("Failed to toggle sync reg rc:%d\n", rc);
> + return rc;
> + }
> +
> + return 0;
> +}
> +
[..]
> +static int qcom_wled_configure(struct qcom_wled *wled, struct device *dev)
> +{
> + struct qcom_wled_config *cfg = &wled->cfg;
> + const __be32 *prop_addr;
> + u32 val, c;
> + int rc, i, j;
> +
> + const struct {
> + const char *name;
> + u32 *val_ptr;
> + const struct qcom_wled_var_cfg *cfg;
> + } u32_opts[] = {
I suggest that you tie this list of options to the compatible (through
of_device_id->data) and pass it as a parameter to this function. That
way you can handle variation in properties and their values between
different compatibles.
[..]
> + *cfg = wled_config_defaults;
> + for (i = 0; i < ARRAY_SIZE(u32_opts); ++i) {
> + rc = of_property_read_u32(dev->of_node, u32_opts[i].name, &val);
of_property_read_u32() returns -ENODATA when there's no associated data,
you can probably use this to implement support for the boolean types in
the same list of opts.
[..]
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(bool_opts); ++i) {
> + if (of_property_read_bool(dev->of_node, bool_opts[i].name))
> + *bool_opts[i].val_ptr = true;
> + }
> +
> + return 0;
> +}
> +
> +static const struct backlight_ops qcom_wled_ops = {
> + .update_status = qcom_wled_update_status,
> + .get_brightness = qcom_wled_get_brightness,
> +};
> +
> +static int qcom_wled_probe(struct platform_device *pdev)
> +{
> + struct backlight_properties props;
> + struct backlight_device *bl;
> + struct qcom_wled *wled;
> + struct regmap *regmap;
> + u32 val;
> + int rc;
> +
> + regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!regmap) {
> + pr_err("Unable to get regmap\n");
> + return -EINVAL;
> + }
> +
> + wled = devm_kzalloc(&pdev->dev, sizeof(*wled), GFP_KERNEL);
> + if (!wled)
> + return -ENOMEM;
> +
> + wled->regmap = regmap;
> + wled->pdev = pdev;
> +
> + rc = qcom_wled_configure(wled, &pdev->dev);
> + if (rc < 0) {
> + pr_err("wled configure failed rc:%d\n", rc);
qcom_wled_configure() already printed an error message for you, no need
to repeat this.
> + return rc;
> + }
> +
Please also run checkpatch.pl with the --strict option and fix the
indentation issues reported.
Regards,
Bjorn
^ permalink raw reply
* Re: [PATCH v2 1/2] of: overlay: Fix memory leak in of_overlay_apply() error path
From: Frank Rowand @ 2017-12-05 2:07 UTC (permalink / raw)
To: Geert Uytterhoeven, Pantelis Antoniou, Rob Herring
Cc: Colin King, Dan Carpenter, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1512402456-8176-2-git-send-email-geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
Hi Geert,
Thanks for finding the issues and for the fixes.
Comments in line.
On 12/04/17 10:47, Geert Uytterhoeven wrote:
> If of_resolve_phandles() fails, free_overlay_changeset() is called in
> the error path. However, that function returns early if the list hasn't
> been initialized yet, before freeing the object.
>
> Explicitly calling kfree() instead would solve that issue. However, that
> complicates matter, by having to consider which of two different methods
> to use to dispose of the same object.
>
> Hence make free_overlay_changeset() consider initialization state of the
> different parts of the object, making it always safe to call (once!) to
> dispose of a (partially) initialized overlay_changeset:
> - Only destroy the changeset if the list was initialized,
> - Ignore uninitialized IDs (zero).
>
> Reported-by: Colin King <colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Fixes: f948d6d8b792bb90 ("of: overlay: avoid race condition between applying multiple overlays")
> Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> ---
> drivers/of/overlay.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 3b7a3980ff50d6bf..312cd658bec0083b 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -630,11 +630,10 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
> {
> int i;
>
> - if (!ovcs->cset.entries.next)
> - return;
> - of_changeset_destroy(&ovcs->cset);
> + if (ovcs->cset.entries.next)
> + of_changeset_destroy(&ovcs->cset);
>
OK
> - if (ovcs->id)
> + if (ovcs->id > 0)
Instead of this change, could you please make a change in init_overlay_changeset()?
Current init_overlay_changeset():
ovcs->id = idr_alloc(&ovcs_idr, ovcs, 1, 0, GFP_KERNEL);
if (ovcs->id <= 0)
return ovcs->id;
My proposed version:
ret = idr_alloc(&ovcs_idr, ovcs, 1, 0, GFP_KERNEL);
if (ret <= 0)
return ret;
ovcs->id = ret;
> idr_remove(&ovcs_idr, ovcs->id);
>
> for (i = 0; i < ovcs->count; i++) {
>
Also, the previous version of the patch, and the discussion around the resulting
bug make me think that I should not have moved 'kfree(ovcs)' into
free_overlay_changeset(), because that kfree is then not very visible in the
error path of of_overlay_apply(). Could you remove 'kfree(ovcs)' from
free_overlay_changeset(), and instead call it immediately after each call
to free_overlay_changeset()?
-Frank
--
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: [PATCH v4] usb: xhci: allow imod-interval to be configurable
From: Chunfeng Yun @ 2017-12-05 2:15 UTC (permalink / raw)
To: Adam Wallis
Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland, Matthias Brugger,
Mathias Nyman, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
timur-sgV2jX0FEOL9JmXXK+q4OQ
In-Reply-To: <1512397671-28733-1-git-send-email-awallis-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On Mon, 2017-12-04 at 09:27 -0500, Adam Wallis wrote:
> The xHCI driver currently has the IMOD set to 160, which
> translates to an IMOD interval of 40,000ns (160 * 250)ns
>
> Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
> introduced a QUIRK for the MTK platform to adjust this interval to 20,
> which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is
> due to the fact that the MTK controller IMOD interval is 8 times
> as much as defined in xHCI spec.
>
> Instead of adding more quirk bits for additional platforms, this patch
> introduces the ability for vendors to set the IMOD_INTERVAL as is
> optimal for their platform. By using device_property_read_u32() on
> "imod-interval-ns", the IMOD INTERVAL can be specified in nano seconds.
> If no interval is specified, the default of 40,000ns (IMOD=160) will be
> used.
>
> No bounds checking has been implemented due to the fact that a vendor
> may have violated the spec and would need to specify a value outside of
> the max 8,000 IRQs/second limit specified in the xHCI spec.
>
> Tested-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Adam Wallis <awallis-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> changes from v3:
> * Changed imod-interval to imod-interval-ns [Rob Herring/Chunfeng]
> * Changed "modulation" to "moderation" throughout patch [Mathias]
> changes from v2:
> * Added PCI default value [Mathias]
> * Removed xhci-mtk.h from xhci-plat.c [Chunfeng Yun]
> * Removed MTK quirk from xhci-plat and moved logic to xhci-mtk [Chunfeng]
> * Updated bindings Documentation to use proper units [Rob Herring]
> * Added imod-interval description and example to MTK binding documentation
> changes from v1:
> * Removed device_property_read_u32() per suggestion from greg k-h
> * Used ER_IRQ_INTERVAL_MASK in place of (u16) cast
>
> Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt | 2 ++
> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
> drivers/usb/host/xhci-mtk.c | 9 +++++++++
> drivers/usb/host/xhci-pci.c | 3 +++
> drivers/usb/host/xhci-plat.c | 5 +++++
> drivers/usb/host/xhci.c | 7 ++-----
> drivers/usb/host/xhci.h | 2 ++
> 7 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
> index 3059596..9ff5602 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
> @@ -46,6 +46,7 @@ Optional properties:
> - pinctrl-names : a pinctrl state named "default" must be defined
> - pinctrl-0 : pin control group
> See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> + - imod-interval-ns: default interrupt moderation interval is 5000ns
>
> Example:
> usb30: usb@11270000 {
> @@ -66,6 +67,7 @@ usb30: usb@11270000 {
> usb3-lpm-capable;
> mediatek,syscon-wakeup = <&pericfg>;
> mediatek,wakeup-src = <1>;
> + imod-interval-ns = <10000>;
> };
>
> 2nd: dual-role mode with xHCI driver
> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> index ae6e484..969908d 100644
> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> @@ -29,6 +29,7 @@ Optional properties:
> - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
> - usb3-lpm-capable: determines if platform is USB3 LPM capable
> - quirk-broken-port-ped: set if the controller has broken port disable mechanism
> + - imod-interval-ns: default interrupt moderation interval is 5000ns
>
> Example:
> usb@f0931000 {
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index b62a1d2..1cb2a8b 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -674,6 +674,15 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>
> xhci = hcd_to_xhci(hcd);
> xhci->main_hcd = hcd;
> +
> + /*
> + * imod_interval is the interrupt moderation value in nanoseconds.
> + * The increment interval is 8 times as much as that defined in
> + * the xHCI spec on MTK's controller.
> + */
> + xhci->imod_interval = 5000;
> + device_property_read_u32(dev, "imod-interval-ns", &xhci->imod_interval);
> +
> xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
> dev_name(dev), hcd);
> if (!xhci->shared_hcd) {
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 7ef1274..4bcddd4 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -234,6 +234,9 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
> if (!xhci->sbrn)
> pci_read_config_byte(pdev, XHCI_SBRN_OFFSET, &xhci->sbrn);
>
> + /* imod_interval is the interrupt moderation value in nanoseconds. */
> + xhci->imod_interval = 40000;
> +
> retval = xhci_gen_setup(hcd, xhci_pci_quirks);
> if (retval)
> return retval;
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 09f164f..6f03830 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -269,6 +269,11 @@ static int xhci_plat_probe(struct platform_device *pdev)
> if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
> xhci->quirks |= XHCI_BROKEN_PORT_PED;
>
> + /* imod_interval is the interrupt moderation value in nanoseconds. */
> + xhci->imod_interval = 40000;
> + device_property_read_u32(sysdev, "imod-interval-ns",
> + &xhci->imod_interval);
> +
> hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
> if (IS_ERR(hcd->usb_phy)) {
> ret = PTR_ERR(hcd->usb_phy);
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 2424d30..0b7755b 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
> "// Set the interrupt modulation register");
s/modulation/moderation
> temp = readl(&xhci->ir_set->irq_control);
> temp &= ~ER_IRQ_INTERVAL_MASK;
> - /*
> - * the increment interval is 8 times as much as that defined
> - * in xHCI spec on MTK's controller
> - */
> - temp |= (u32) ((xhci->quirks & XHCI_MTK_HOST) ? 20 : 160);
> + temp |= (xhci->imod_interval / 250) & ER_IRQ_INTERVAL_MASK;
> +
No need a blank line
> writel(temp, &xhci->ir_set->irq_control);
>
> /* Set the HCD state before we enable the irqs */
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 99a014a..2a4177b 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1717,6 +1717,8 @@ struct xhci_hcd {
> u8 max_interrupters;
> u8 max_ports;
> u8 isoc_threshold;
> + /* imod_interval in ns (I * 250ns) */
> + u32 imod_interval;
> int event_ring_max;
> /* 4KB min, 128MB max */
> int page_size;
Thanks
--
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: [PATCH v2 2/2] of: overlay: Fix cleanup order in of_overlay_apply()
From: Rob Herring @ 2017-12-05 2:25 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Geert Uytterhoeven, Pantelis Antoniou, Frank Rowand, Colin King,
Dan Carpenter, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAMuHMdVFR3FPVAM2J_L2HvNUzkJsQM57SJJ2CMux71M4dhZYfQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, Dec 4, 2017 at 1:45 PM, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> Hi Rob,
>
> On Mon, Dec 4, 2017 at 8:35 PM, Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Mon, Dec 4, 2017 at 9:47 AM, Geert Uytterhoeven
>> <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org> wrote:
>>> The special overlay mutex is taken first, hence it should be released
>>> last in the error path.
>>>
>>> Move "mutex_lock(&of_mutex)" up, as suggested by Frank, as
>>> free_overlay_changeset() should be called with that mutex held if any
>>> non-trivial cleanup is to be done.
>>
>> Not holding the of_mutex for of_resolve_phandles is just wrong.
>> Without it, a node and new phandle could be added via of_attach_node
>> making the max phandle wrong.
>
> After my patch it's held, so what's the problem?
There's no problem. Just highlighting the issue with the prior
location is more than it seems from your explanation.
Rob
--
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: [PATCH v3 2/3] mailbox: Add support for Hi3660 mailbox
From: Leo Yan @ 2017-12-05 2:29 UTC (permalink / raw)
To: Xu YiPing
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
xuwei5-C8/M+/jPZTeaMJb+Lgu22Q, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
guodong.xu-QSEj5FYQhm4dnm+yROfE0A,
haojian.zhuang-QSEj5FYQhm4dnm+yROfE0A,
suzhuangluan-C8/M+/jPZTeaMJb+Lgu22Q,
xuezhiliang-C8/M+/jPZTeaMJb+Lgu22Q,
kevin.wangtao-C8/M+/jPZTeaMJb+Lgu22Q,
zhongkaihua-hv44wF8Li93QT0dZR+AlfA
In-Reply-To: <1510910672-1409-3-git-send-email-xuyiping-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
On Fri, Nov 17, 2017 at 05:24:31PM +0800, Xu YiPing wrote:
> From: Kaihua Zhong <zhongkaihua-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>
> Hi3660 mailbox controller is used to send message within multiple
> processors, MCU, HIFI, etc. It supports 32 mailbox channels and every
> channel can only be used for single transferring direction. Once the
> channel is enabled, it needs to specify the destination interrupt and
> acknowledge interrupt, these two interrupt vectors are used to create
> the connection between the mailbox and interrupt controllers.
>
> The data transferring supports two modes, one is named as "automatic
> acknowledge" mode so after send message the kernel doesn't need to wait
> for acknowledge from remote and directly return; there have another mode
> is to rely on handling interrupt for acknowledge.
>
> This commit is for initial version driver, which only supports
> "automatic acknowledge" mode to support CPU clock, which is the only
> one consumer to use mailbox and has been verified. Later may enhance
> this driver for interrupt mode (e.g. for supporting HIFI).
Gentle ping ...
> Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Ruyi Wang <wangruyi-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Kaihua Zhong <zhongkaihua-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
> drivers/mailbox/Kconfig | 8 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/hi3660-mailbox.c | 322 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 332 insertions(+)
> create mode 100644 drivers/mailbox/hi3660-mailbox.c
>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index c5731e5..4b5d6e9 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -108,6 +108,14 @@ config TI_MESSAGE_MANAGER
> multiple processors within the SoC. Select this driver if your
> platform has support for the hardware block.
>
> +config HI3660_MBOX
> + tristate "Hi3660 Mailbox"
> + depends on ARCH_HISI && OF
> + help
> + An implementation of the hi3660 mailbox. It is used to send message
> + between application processors and other processors/MCU/DSP. Select
> + Y here if you want to use Hi3660 mailbox controller.
> +
> config HI6220_MBOX
> tristate "Hi6220 Mailbox"
> depends on ARCH_HISI
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index d54e412..7d1bd51 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o
>
> obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o
>
> +obj-$(CONFIG_HI3660_MBOX) += hi3660-mailbox.o
> +
> obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o
>
> obj-$(CONFIG_BCM_PDC_MBOX) += bcm-pdc-mailbox.o
> diff --git a/drivers/mailbox/hi3660-mailbox.c b/drivers/mailbox/hi3660-mailbox.c
> new file mode 100644
> index 0000000..ba80834
> --- /dev/null
> +++ b/drivers/mailbox/hi3660-mailbox.c
> @@ -0,0 +1,322 @@
> +/*
> + * Hisilicon's Hi3660 mailbox controller driver
> + *
> + * Copyright (c) 2017 Hisilicon Limited.
> + * Copyright (c) 2017 Linaro Limited.
> + *
> + * Author: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "mailbox.h"
> +
> +#define MBOX_CHAN_MAX 32
> +
> +#define MBOX_RX (0x0)
> +#define MBOX_TX (0x1)
> +
> +#define MBOX_BASE(mbox, ch) ((mbox)->base + ((ch) * 0x40))
> +#define MBOX_SRC_REG (0x00)
> +#define MBOX_DST_REG (0x04)
> +#define MBOX_DCLR_REG (0x08)
> +#define MBOX_DSTAT_REG (0x0c)
> +#define MBOX_MODE_REG (0x10)
> +#define MBOX_IMASK_REG (0x14)
> +#define MBOX_ICLR_REG (0x18)
> +#define MBOX_SEND_REG (0x1c)
> +#define MBOX_DATA_REG (0x20)
> +
> +#define MBOX_IPC_LOCK_REG (0xa00)
> +#define MBOX_IPC_UNLOCK (0x1acce551)
> +
> +#define MBOX_AUTOMATIC_ACK (1)
> +
> +#define MBOX_STATE_IDLE BIT(4)
> +#define MBOX_STATE_ACK BIT(7)
> +
> +#define MBOX_MSG_LEN 8
> +
> +/**
> + * Hi3660 mailbox channel device data
> + *
> + * A channel can be used for TX or RX, it can trigger remote
> + * processor interrupt to notify remote processor and can receive
> + * interrupt if has incoming message.
> + *
> + * @dst_irq: Interrupt vector for remote processor
> + * @ack_irq: Interrupt vector for local processor
> + */
> +struct hi3660_mbox_dev {
> + unsigned int dst_irq;
> + unsigned int ack_irq;
> +};
> +
> +/**
> + * Hi3660 mailbox controller data
> + *
> + * Mailbox controller includes 32 channels and can allocate
> + * channel for message transferring.
> + *
> + * @dev: Device to which it is attached
> + * @base: Base address of the register mapping region
> + * @chan: Representation of channels in mailbox controller
> + * @mdev: Representation of channel device data
> + * @controller: Representation of a communication channel controller
> + */
> +struct hi3660_mbox {
> + struct device *dev;
> + void __iomem *base;
> + struct mbox_chan chan[MBOX_CHAN_MAX];
> + struct hi3660_mbox_dev mdev[MBOX_CHAN_MAX];
> + struct mbox_controller controller;
> +};
> +
> +static inline struct hi3660_mbox *to_hi3660_mbox(struct mbox_controller *mbox)
> +{
> + return container_of(mbox, struct hi3660_mbox, controller);
> +}
> +
> +static int hi3660_mbox_check_state(struct mbox_chan *chan)
> +{
> + unsigned long ch = (unsigned long)chan->con_priv;
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> + void __iomem *base = MBOX_BASE(mbox, ch);
> + unsigned long val;
> + unsigned int state, ret;
> +
> + /* Mailbox is idle so directly bail out */
> + state = readl_relaxed(base + MBOX_MODE_REG);
> + if (state & MBOX_STATE_IDLE)
> + return 0;
> +
> + /* Wait for acknowledge from remote */
> + ret = readx_poll_timeout_atomic(readl_relaxed, base + MBOX_MODE_REG,
> + val, (val & MBOX_STATE_ACK), 1000, 300000);
> + if (ret) {
> + dev_err(mbox->dev, "%s: timeout for receiving ack\n", __func__);
> + return ret;
> + }
> +
> + /* Ensure channel is released */
> + writel_relaxed(0xffffffff, base + MBOX_IMASK_REG);
> + writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> +
> + return 0;
> +}
> +
> +static int hi3660_mbox_unlock(struct mbox_chan *chan)
> +{
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + unsigned int val, retry = 3;
> +
> + do {
> + writel_relaxed(MBOX_IPC_UNLOCK, mbox->base + MBOX_IPC_LOCK_REG);
> +
> + val = readl_relaxed(mbox->base + MBOX_IPC_LOCK_REG);
> + if (!val)
> + break;
> +
> + udelay(10);
> + } while (retry--);
> +
> + return (!val) ? 0 : -ETIMEDOUT;
> +}
> +
> +static int hi3660_mbox_acquire_channel(struct mbox_chan *chan)
> +{
> + unsigned long ch = (unsigned long)chan->con_priv;
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> + void __iomem *base = MBOX_BASE(mbox, ch);
> + unsigned int val, retry;
> +
> + for (retry = 10; retry; retry--) {
> + /* Check if channel has been acquired */
> + if (readl_relaxed(base + MBOX_MODE_REG) & MBOX_STATE_IDLE) {
> + writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> + val = readl_relaxed(base + MBOX_SRC_REG);
> + if (val & BIT(mdev->ack_irq))
> + break;
> + }
> + }
> +
> + return retry ? 0 : -ETIMEDOUT;
> +}
> +
> +static int hi3660_mbox_send(struct mbox_chan *chan, u32 *msg)
> +{
> + unsigned long ch = (unsigned long)chan->con_priv;
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> + void __iomem *base = MBOX_BASE(mbox, ch);
> + unsigned int i;
> +
> + /* Clear mask for destination interrupt */
> + writel_relaxed(~BIT(mdev->dst_irq), base + MBOX_IMASK_REG);
> +
> + /* Config destination for interrupt vector */
> + writel_relaxed(BIT(mdev->dst_irq), base + MBOX_DST_REG);
> +
> + /* Automatic acknowledge mode */
> + writel_relaxed(MBOX_AUTOMATIC_ACK, base + MBOX_MODE_REG);
> +
> + /* Fill message data */
> + for (i = 0; i < MBOX_MSG_LEN; i++)
> + writel_relaxed(msg[i], base + MBOX_DATA_REG + i * 4);
> +
> + /* Trigger data transferring */
> + writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SEND_REG);
> + return 0;
> +}
> +
> +static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)
> +{
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + int err;
> +
> + err = hi3660_mbox_check_state(chan);
> + if (err) {
> + dev_err(mbox->dev, "checking state failed\n");
> + return err;
> + }
> +
> + err = hi3660_mbox_unlock(chan);
> + if (err) {
> + dev_err(mbox->dev, "unlocking mailbox failed\n");
> + return err;
> + }
> +
> + err = hi3660_mbox_acquire_channel(chan);
> + if (err) {
> + dev_err(mbox->dev, "acquiring channel failed\n");
> + return err;
> + }
> +
> + return hi3660_mbox_send(chan, msg);
> +}
> +
> +static struct mbox_chan_ops hi3660_mbox_ops = {
> + .send_data = hi3660_mbox_send_data,
> +};
> +
> +static struct mbox_chan *hi3660_mbox_xlate(struct mbox_controller *controller,
> + const struct of_phandle_args *spec)
> +{
> + struct hi3660_mbox *mbox = to_hi3660_mbox(controller);
> + struct hi3660_mbox_dev *mdev;
> + unsigned int ch = spec->args[0];
> +
> + if (ch >= MBOX_CHAN_MAX) {
> + dev_err(mbox->dev, "Invalid channel idx %d\n", ch);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + mdev = &mbox->mdev[ch];
> + mdev->dst_irq = spec->args[1];
> + mdev->ack_irq = spec->args[2];
> +
> + return &mbox->chan[ch];
> +}
> +
> +static const struct of_device_id hi3660_mbox_of_match[] = {
> + { .compatible = "hisilicon,hi3660-mbox", },
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, hi3660_mbox_of_match);
> +
> +static int hi3660_mbox_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct hi3660_mbox *mbox;
> + struct mbox_chan *chan;
> + struct resource *res;
> + unsigned long ch;
> + int err;
> +
> + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> + if (!mbox)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + mbox->base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(mbox->base))
> + return PTR_ERR(mbox->base);
> +
> + mbox->dev = dev;
> + mbox->controller.dev = dev;
> + mbox->controller.chans = mbox->chan;
> + mbox->controller.num_chans = MBOX_CHAN_MAX;
> + mbox->controller.ops = &hi3660_mbox_ops;
> + mbox->controller.of_xlate = hi3660_mbox_xlate;
> +
> + /* Initialize mailbox channel data */
> + chan = mbox->chan;
> + for (ch = 0; ch < MBOX_CHAN_MAX; ch++)
> + chan[ch].con_priv = (void *)ch;
> +
> + err = mbox_controller_register(&mbox->controller);
> + if (err) {
> + dev_err(dev, "Failed to register mailbox %d\n", err);
> + return err;
> + }
> +
> + platform_set_drvdata(pdev, mbox);
> + dev_info(dev, "Mailbox enabled\n");
> + return 0;
> +}
> +
> +static int hi3660_mbox_remove(struct platform_device *pdev)
> +{
> + struct hi3660_mbox *mbox = platform_get_drvdata(pdev);
> +
> + mbox_controller_unregister(&mbox->controller);
> + return 0;
> +}
> +
> +static struct platform_driver hi3660_mbox_driver = {
> + .probe = hi3660_mbox_probe,
> + .remove = hi3660_mbox_remove,
> + .driver = {
> + .name = "hi3660-mbox",
> + .of_match_table = hi3660_mbox_of_match,
> + },
> +};
> +
> +static int __init hi3660_mbox_init(void)
> +{
> + return platform_driver_register(&hi3660_mbox_driver);
> +}
> +core_initcall(hi3660_mbox_init);
> +
> +static void __exit hi3660_mbox_exit(void)
> +{
> + platform_driver_unregister(&hi3660_mbox_driver);
> +}
> +module_exit(hi3660_mbox_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Hisilicon Hi3660 Mailbox Controller");
> +MODULE_AUTHOR("Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>");
> --
> 1.9.1
>
--
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: [PATCH v3 2/3] clk: hisilicon: Add support for Hi3660 stub clocks
From: Leo Yan @ 2017-12-05 2:33 UTC (permalink / raw)
To: Xu YiPing
Cc: mturquette, sboyd, robh+dt, mark.rutland, xuwei5, catalin.marinas,
will.deacon, xuejiancheng, wenpan, zhangfei.gao, guodong.xu,
zhongkaihua, chenjun14, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, suzhuangluan, xuezhiliang, kevin.wangtao
In-Reply-To: <1510910852-2175-3-git-send-email-xuyiping@hisilicon.com>
On Fri, Nov 17, 2017 at 05:27:31PM +0800, Xu YiPing wrote:
> From: Kaihua Zhong <zhongkaihua@huawei.com>
>
> Hi3660 has four stub clocks, which are big and LITTLE cluster clocks,
> GPU clock and DDR clock. These clocks ask MCU for frequency scaling
> by sending message through mailbox.
>
> This commit adds support for stub clocks, it requests the dedicated
> mailbox channel at initialization; then later uses this channel to send
> message to MCU to execute frequency scaling. The four stub clocks share
> the same mailbox channel, but every stub clock has its own command id so
> MCU can distinguish the requirement coming for which clock.
>
> A shared memory is used to present effective frequency value, so the
> clock driver uses I/O mapping for the memory and reads back rate value.
Hi Michael, Stephen,
Could you help review this patch?
> Reviewed-by: Leo Yan <leo.yan@linaro.org>
> Signed-off-by: Kai Zhao <zhaokai1@hisilicon.com>
> Signed-off-by: Tao Wang <kevin.wangtao@hisilicon.com>
> Signed-off-by: Ruyi Wang <wangruyi@huawei.com>
> Signed-off-by: Kaihua Zhong <zhongkaihua@huawei.com>
> ---
> drivers/clk/hisilicon/Kconfig | 6 ++
> drivers/clk/hisilicon/Makefile | 1 +
> drivers/clk/hisilicon/clk-hi3660-stub.c | 186 ++++++++++++++++++++++++++++++++
> 3 files changed, 193 insertions(+)
> create mode 100644 drivers/clk/hisilicon/clk-hi3660-stub.c
>
> diff --git a/drivers/clk/hisilicon/Kconfig b/drivers/clk/hisilicon/Kconfig
> index 7098bfd..1bd4355 100644
> --- a/drivers/clk/hisilicon/Kconfig
> +++ b/drivers/clk/hisilicon/Kconfig
> @@ -49,3 +49,9 @@ config STUB_CLK_HI6220
> default ARCH_HISI
> help
> Build the Hisilicon Hi6220 stub clock driver.
> +
> +config STUB_CLK_HI3660
> + bool "Hi3660 Stub Clock Driver"
> + depends on COMMON_CLK_HI3660 && MAILBOX
> + help
> + Build the Hisilicon Hi3660 stub clock driver.
> diff --git a/drivers/clk/hisilicon/Makefile b/drivers/clk/hisilicon/Makefile
> index 1e4c3dd..0a5b499 100644
> --- a/drivers/clk/hisilicon/Makefile
> +++ b/drivers/clk/hisilicon/Makefile
> @@ -14,3 +14,4 @@ obj-$(CONFIG_COMMON_CLK_HI3798CV200) += crg-hi3798cv200.o
> obj-$(CONFIG_COMMON_CLK_HI6220) += clk-hi6220.o
> obj-$(CONFIG_RESET_HISI) += reset.o
> obj-$(CONFIG_STUB_CLK_HI6220) += clk-hi6220-stub.o
> +obj-$(CONFIG_STUB_CLK_HI3660) += clk-hi3660-stub.o
> diff --git a/drivers/clk/hisilicon/clk-hi3660-stub.c b/drivers/clk/hisilicon/clk-hi3660-stub.c
> new file mode 100644
> index 0000000..607efa4
> --- /dev/null
> +++ b/drivers/clk/hisilicon/clk-hi3660-stub.c
> @@ -0,0 +1,186 @@
> +/*
> + * Hisilicon clock driver
> + *
> + * Copyright (c) 2013-2017 Hisilicon Limited.
> + * Copyright (c) 2017 Linaro Limited.
> + *
> + * Author: Kai Zhao <zhaokai1@hisilicon.com>
> + * Tao Wang <kevin.wangtao@hisilicon.com>
> + * Leo Yan <leo.yan@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/mailbox_client.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <dt-bindings/clock/hi3660-clock.h>
> +
> +#define HI3660_STUB_CLOCK_DATA (0x70)
> +#define MHZ (1000 * 1000)
> +
> +#define DEFINE_CLK_STUB(_id, _cmd, _name) \
> + { \
> + .id = (_id), \
> + .cmd = (_cmd), \
> + .hw.init = &(struct clk_init_data) { \
> + .name = #_name, \
> + .ops = &hi3660_stub_clk_ops, \
> + .num_parents = 0, \
> + .flags = CLK_GET_RATE_NOCACHE, \
> + }, \
> + },
> +
> +#define to_stub_clk(_hw) container_of(_hw, struct hi3660_stub_clk, hw)
> +
> +struct hi3660_stub_clk_chan {
> + struct mbox_client cl;
> + struct mbox_chan *mbox;
> +};
> +
> +struct hi3660_stub_clk {
> + unsigned int id;
> + struct clk_hw hw;
> + unsigned int cmd;
> + unsigned int msg[8];
> + unsigned int rate;
> +};
> +
> +static void __iomem *freq_reg;
> +static struct hi3660_stub_clk_chan stub_clk_chan;
> +
> +static unsigned long hi3660_stub_clk_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct hi3660_stub_clk *stub_clk = to_stub_clk(hw);
> +
> + /*
> + * LPM3 writes back the CPU frequency in shared SRAM so read
> + * back the frequency.
> + */
> + stub_clk->rate = readl(freq_reg + (stub_clk->id << 2)) * MHZ;
> + return stub_clk->rate;
> +}
> +
> +static long hi3660_stub_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *prate)
> +{
> + /*
> + * LPM3 handles rate rounding so just return whatever
> + * rate is requested.
> + */
> + return rate;
> +}
> +
> +static int hi3660_stub_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct hi3660_stub_clk *stub_clk = to_stub_clk(hw);
> +
> + stub_clk->msg[0] = stub_clk->cmd;
> + stub_clk->msg[1] = rate / MHZ;
> +
> + dev_dbg(stub_clk_chan.cl.dev, "set rate msg[0]=0x%x msg[1]=0x%x\n",
> + stub_clk->msg[0], stub_clk->msg[1]);
> +
> + mbox_send_message(stub_clk_chan.mbox, stub_clk->msg);
> + mbox_client_txdone(stub_clk_chan.mbox, 0);
> +
> + stub_clk->rate = rate;
> + return 0;
> +}
> +
> +static const struct clk_ops hi3660_stub_clk_ops = {
> + .recalc_rate = hi3660_stub_clk_recalc_rate,
> + .round_rate = hi3660_stub_clk_round_rate,
> + .set_rate = hi3660_stub_clk_set_rate,
> +};
> +
> +static struct hi3660_stub_clk hi3660_stub_clks[HI3660_CLK_STUB_NUM] = {
> + DEFINE_CLK_STUB(HI3660_CLK_STUB_CLUSTER0, 0x0001030A, "cpu-cluster.0")
> + DEFINE_CLK_STUB(HI3660_CLK_STUB_CLUSTER1, 0x0002030A, "cpu-cluster.1")
> + DEFINE_CLK_STUB(HI3660_CLK_STUB_GPU, 0x0003030A, "clk-g3d")
> + DEFINE_CLK_STUB(HI3660_CLK_STUB_DDR, 0x00040309, "clk-ddrc")
> +};
> +
> +static struct clk_hw *hi3660_stub_clk_hw_get(struct of_phandle_args *clkspec,
> + void *data)
> +{
> + unsigned int idx = clkspec->args[0];
> +
> + if (idx > HI3660_CLK_STUB_NUM) {
> + pr_err("%s: invalid index %u\n", __func__, idx);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return &hi3660_stub_clks[idx].hw;
> +}
> +
> +static int hi3660_stub_clk_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + unsigned int i;
> + int ret;
> +
> + /* Use mailbox client without blocking */
> + stub_clk_chan.cl.dev = dev;
> + stub_clk_chan.cl.tx_done = NULL;
> + stub_clk_chan.cl.tx_block = false;
> + stub_clk_chan.cl.knows_txdone = false;
> +
> + /* Allocate mailbox channel */
> + stub_clk_chan.mbox = mbox_request_channel(&stub_clk_chan.cl, 0);
> + if (IS_ERR(stub_clk_chan.mbox))
> + return PTR_ERR(stub_clk_chan.mbox);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + freq_reg = devm_ioremap(dev, res->start, resource_size(res));
> + if (IS_ERR(freq_reg))
> + return -ENOMEM;
> +
> + freq_reg += HI3660_STUB_CLOCK_DATA;
> +
> + for (i = 0; i < HI3660_CLK_STUB_NUM; i++) {
> + ret = devm_clk_hw_register(&pdev->dev, &hi3660_stub_clks[i].hw);
> + if (ret)
> + return ret;
> + }
> +
> + ret = of_clk_add_hw_provider(pdev->dev.of_node, hi3660_stub_clk_hw_get,
> + hi3660_stub_clks);
> + return ret;
> +}
> +
> +static const struct of_device_id hi3660_stub_clk_of_match[] = {
> + { .compatible = "hisilicon,hi3660-stub-clk", },
> + {}
> +};
> +
> +static struct platform_driver hi3660_stub_clk_driver = {
> + .probe = hi3660_stub_clk_probe,
> + .driver = {
> + .name = "hi3660-stub-clk",
> + .of_match_table = hi3660_stub_clk_of_match,
> + },
> +};
> +
> +static int __init hi3660_stub_clk_init(void)
> +{
> + return platform_driver_register(&hi3660_stub_clk_driver);
> +}
> +subsys_initcall(hi3660_stub_clk_init);
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH] Documentation: binding: Update endianness usage
From: Scott Wood @ 2017-12-05 2:45 UTC (permalink / raw)
To: Prabhakar Kushwaha, linux-mtd@lists.infradead.org, devicetree
Cc: computersforpeace@gmail.com, dedekind1@gmail.com
In-Reply-To: <HE1PR04MB1241226CD3B7ABD6D5FA2D91973C0@HE1PR04MB1241.eurprd04.prod.outlook.com>
On Mon, 2017-12-04 at 04:33 +0000, Prabhakar Kushwaha wrote:
> > -----Original Message-----
> > From: Scott Wood [mailto:oss@buserror.net]
> > Sent: Saturday, December 02, 2017 3:25 AM
> > To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
> > mtd@lists.infradead.org; devicetree-discuss@lists.ozlabs.org
> > Cc: dedekind1@gmail.com; computersforpeace@gmail.com
> > Subject: Re: [PATCH] Documentation: binding: Update endianness usage
> >
> > On Fri, 2017-12-01 at 08:42 +0000, Prabhakar Kushwaha wrote:
> > > > -----Original Message-----
> > > > From: Scott Wood [mailto:oss@buserror.net]
> > > > Sent: Friday, December 01, 2017 10:43 AM
> > > > To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; linux-
> > > > mtd@lists.infradead.org; devicetree-discuss@lists.ozlabs.org
> > > > Cc: dedekind1@gmail.com; computersforpeace@gmail.com
> > > > Subject: Re: [PATCH] Documentation: binding: Update endianness usage
> > > >
> > > > If big endian is the default, is this change really
> > > > necessary? Particularly
> > > > since the big endian chips are older and thus have existing device
> > > > trees.
> > > >
> > >
> > > Earlier endianness information was only used for "how to" access IFC-
> > > NAND
> > > register access.
> > > Now this info will also be used for defining swap requirement of NOR
> > > flash.
> >
> > Is this a difference between LS1021A and PPC-based chips?
> >
>
> Yes.
> CONFIG_MTD_CFI_BE_BYTE_SWAP needs to be defined For LS1021A, LS1043A,
> LS1046A
Only because you're running a little-endian kernel on those chips. I still
don't see why the absence of a little-endian property isn't sufficient to
communicate that the hardware is big-endian given that that's the established
default.
I now see your patch to of_flash_probe... where is the non-IFC-specific
binding that says the *parent* of a CFI node should be looked at for this?
Where in general are endian properties kept in the parent of the node with
"reg"? The right answer is to add endianness to mtd-physmap.txt.
-Scott
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* Re: [PATCH v4] usb: xhci: allow imod-interval to be configurable
From: Adam Wallis @ 2017-12-05 2:54 UTC (permalink / raw)
To: Chunfeng Yun
Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland, Matthias Brugger,
Mathias Nyman, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
timur-sgV2jX0FEOL9JmXXK+q4OQ
In-Reply-To: <1512440100.17567.172.camel@mhfsdcap03>
On 12/4/2017 9:15 PM, Chunfeng Yun wrote:
> On Mon, 2017-12-04 at 09:27 -0500, Adam Wallis wrote:
>> The xHCI driver currently has the IMOD set to 160, which
>> translates to an IMOD interval of 40,000ns (160 * 250)ns
>>
>> Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
>> introduced a QUIRK for the MTK platform to adjust this interval to 20,
>> which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is
>> due to the fact that the MTK controller IMOD interval is 8 times
>> as much as defined in xHCI spec.
>>
>> Instead of adding more quirk bits for additional platforms, this patch
>> introduces the ability for vendors to set the IMOD_INTERVAL as is
>> optimal for their platform. By using device_property_read_u32() on
>> "imod-interval-ns", the IMOD INTERVAL can be specified in nano seconds.
>> If no interval is specified, the default of 40,000ns (IMOD=160) will be
>> used.
>>
>> No bounds checking has been implemented due to the fact that a vendor
>> may have violated the spec and would need to specify a value outside of
>> the max 8,000 IRQs/second limit specified in the xHCI spec.
>>
>> Tested-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Adam Wallis <awallis-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>> changes from v3:
>> * Changed imod-interval to imod-interval-ns [Rob Herring/Chunfeng]
>> * Changed "modulation" to "moderation" throughout patch [Mathias]
>> changes from v2:
>> * Added PCI default value [Mathias]
>> * Removed xhci-mtk.h from xhci-plat.c [Chunfeng Yun]
>> * Removed MTK quirk from xhci-plat and moved logic to xhci-mtk [Chunfeng]
>> * Updated bindings Documentation to use proper units [Rob Herring]
>> * Added imod-interval description and example to MTK binding documentation
>> changes from v1:
>> * Removed device_property_read_u32() per suggestion from greg k-h
>> * Used ER_IRQ_INTERVAL_MASK in place of (u16) cast
>>
>> Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt | 2 ++
>> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
>> drivers/usb/host/xhci-mtk.c | 9 +++++++++
>> drivers/usb/host/xhci-pci.c | 3 +++
>> drivers/usb/host/xhci-plat.c | 5 +++++
>> drivers/usb/host/xhci.c | 7 ++-----
>> drivers/usb/host/xhci.h | 2 ++
>> 7 files changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
>> index 3059596..9ff5602 100644
>> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
>> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
>> @@ -46,6 +46,7 @@ Optional properties:
>> - pinctrl-names : a pinctrl state named "default" must be defined
>> - pinctrl-0 : pin control group
>> See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>> + - imod-interval-ns: default interrupt moderation interval is 5000ns
>>
>> Example:
>> usb30: usb@11270000 {
>> @@ -66,6 +67,7 @@ usb30: usb@11270000 {
>> usb3-lpm-capable;
>> mediatek,syscon-wakeup = <&pericfg>;
>> mediatek,wakeup-src = <1>;
>> + imod-interval-ns = <10000>;
>> };
>>
>> 2nd: dual-role mode with xHCI driver
>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> index ae6e484..969908d 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> @@ -29,6 +29,7 @@ Optional properties:
>> - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
>> - usb3-lpm-capable: determines if platform is USB3 LPM capable
>> - quirk-broken-port-ped: set if the controller has broken port disable mechanism
>> + - imod-interval-ns: default interrupt moderation interval is 5000ns
>>
>> Example:
>> usb@f0931000 {
>> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
>> index b62a1d2..1cb2a8b 100644
>> --- a/drivers/usb/host/xhci-mtk.c
>> +++ b/drivers/usb/host/xhci-mtk.c
>> @@ -674,6 +674,15 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>>
>> xhci = hcd_to_xhci(hcd);
>> xhci->main_hcd = hcd;
>> +
>> + /*
>> + * imod_interval is the interrupt moderation value in nanoseconds.
>> + * The increment interval is 8 times as much as that defined in
>> + * the xHCI spec on MTK's controller.
>> + */
>> + xhci->imod_interval = 5000;
>> + device_property_read_u32(dev, "imod-interval-ns", &xhci->imod_interval);
>> +
>> xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
>> dev_name(dev), hcd);
>> if (!xhci->shared_hcd) {
>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>> index 7ef1274..4bcddd4 100644
>> --- a/drivers/usb/host/xhci-pci.c
>> +++ b/drivers/usb/host/xhci-pci.c
>> @@ -234,6 +234,9 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
>> if (!xhci->sbrn)
>> pci_read_config_byte(pdev, XHCI_SBRN_OFFSET, &xhci->sbrn);
>>
>> + /* imod_interval is the interrupt moderation value in nanoseconds. */
>> + xhci->imod_interval = 40000;
>> +
>> retval = xhci_gen_setup(hcd, xhci_pci_quirks);
>> if (retval)
>> return retval;
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 09f164f..6f03830 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -269,6 +269,11 @@ static int xhci_plat_probe(struct platform_device *pdev)
>> if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
>> xhci->quirks |= XHCI_BROKEN_PORT_PED;
>>
>> + /* imod_interval is the interrupt moderation value in nanoseconds. */
>> + xhci->imod_interval = 40000;
>> + device_property_read_u32(sysdev, "imod-interval-ns",
>> + &xhci->imod_interval);
>> +
>> hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
>> if (IS_ERR(hcd->usb_phy)) {
>> ret = PTR_ERR(hcd->usb_phy);
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index 2424d30..0b7755b 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
>> "// Set the interrupt modulation register");
> s/modulation/moderation
Mathias said there was no need to change the existing modulation strings - only
the ones that I had added.
>
>> temp = readl(&xhci->ir_set->irq_control);
>> temp &= ~ER_IRQ_INTERVAL_MASK;
>> - /*
>> - * the increment interval is 8 times as much as that defined
>> - * in xHCI spec on MTK's controller
>> - */
>> - temp |= (u32) ((xhci->quirks & XHCI_MTK_HOST) ? 20 : 160);
>> + temp |= (xhci->imod_interval / 250) & ER_IRQ_INTERVAL_MASK;
>> +
> No need a blank line
If this patch goes through another version, I will remove this line
>
>> writel(temp, &xhci->ir_set->irq_control);
>>
>> /* Set the HCD state before we enable the irqs */
>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>> index 99a014a..2a4177b 100644
>> --- a/drivers/usb/host/xhci.h
>> +++ b/drivers/usb/host/xhci.h
>> @@ -1717,6 +1717,8 @@ struct xhci_hcd {
>> u8 max_interrupters;
>> u8 max_ports;
>> u8 isoc_threshold;
>> + /* imod_interval in ns (I * 250ns) */
>> + u32 imod_interval;
>> int event_ring_max;
>> /* 4KB min, 128MB max */
>> int page_size;
>
> Thanks
>
>
--
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
--
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: [PATCH v3 2/3] mailbox: Add support for Hi3660 mailbox
From: Jassi Brar @ 2017-12-05 3:28 UTC (permalink / raw)
To: Xu YiPing
Cc: Rob Herring, Mark Rutland, Wei Xu, Catalin Marinas, Will Deacon,
Devicetree List, Linux Kernel Mailing List,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Guodong Xu, Haojian Zhuang, suzhuangluan-C8/M+/jPZTeaMJb+Lgu22Q,
xuezhiliang-C8/M+/jPZTeaMJb+Lgu22Q,
kevin.wangtao-C8/M+/jPZTeaMJb+Lgu22Q, Zhong Kaihua
In-Reply-To: <1510910672-1409-3-git-send-email-xuyiping-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
On Fri, Nov 17, 2017 at 2:54 PM, Xu YiPing <xuyiping-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org> wrote:
> From: Kaihua Zhong <zhongkaihua-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>
> Hi3660 mailbox controller is used to send message within multiple
> processors, MCU, HIFI, etc. It supports 32 mailbox channels and every
> channel can only be used for single transferring direction. Once the
> channel is enabled, it needs to specify the destination interrupt and
> acknowledge interrupt, these two interrupt vectors are used to create
> the connection between the mailbox and interrupt controllers.
>
> The data transferring supports two modes, one is named as "automatic
> acknowledge" mode so after send message the kernel doesn't need to wait
> for acknowledge from remote and directly return; there have another mode
> is to rely on handling interrupt for acknowledge.
>
> This commit is for initial version driver, which only supports
> "automatic acknowledge" mode to support CPU clock, which is the only
> one consumer to use mailbox and has been verified. Later may enhance
> this driver for interrupt mode (e.g. for supporting HIFI).
>
> Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Ruyi Wang <wangruyi-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Kaihua Zhong <zhongkaihua-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
> drivers/mailbox/Kconfig | 8 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/hi3660-mailbox.c | 322 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 332 insertions(+)
> create mode 100644 drivers/mailbox/hi3660-mailbox.c
>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index c5731e5..4b5d6e9 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -108,6 +108,14 @@ config TI_MESSAGE_MANAGER
> multiple processors within the SoC. Select this driver if your
> platform has support for the hardware block.
>
> +config HI3660_MBOX
> + tristate "Hi3660 Mailbox"
> + depends on ARCH_HISI && OF
> + help
> + An implementation of the hi3660 mailbox. It is used to send message
> + between application processors and other processors/MCU/DSP. Select
> + Y here if you want to use Hi3660 mailbox controller.
> +
> config HI6220_MBOX
> tristate "Hi6220 Mailbox"
> depends on ARCH_HISI
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index d54e412..7d1bd51 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o
>
> obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o
>
> +obj-$(CONFIG_HI3660_MBOX) += hi3660-mailbox.o
> +
> obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o
>
> obj-$(CONFIG_BCM_PDC_MBOX) += bcm-pdc-mailbox.o
> diff --git a/drivers/mailbox/hi3660-mailbox.c b/drivers/mailbox/hi3660-mailbox.c
> new file mode 100644
> index 0000000..ba80834
> --- /dev/null
> +++ b/drivers/mailbox/hi3660-mailbox.c
> @@ -0,0 +1,322 @@
> +/*
> + * Hisilicon's Hi3660 mailbox controller driver
> + *
> + * Copyright (c) 2017 Hisilicon Limited.
> + * Copyright (c) 2017 Linaro Limited.
> + *
> + * Author: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
>
We now use SPDX licence
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "mailbox.h"
> +
> +#define MBOX_CHAN_MAX 32
> +
> +#define MBOX_RX (0x0)
> +#define MBOX_TX (0x1)
> +
> +#define MBOX_BASE(mbox, ch) ((mbox)->base + ((ch) * 0x40))
> +#define MBOX_SRC_REG (0x00)
> +#define MBOX_DST_REG (0x04)
> +#define MBOX_DCLR_REG (0x08)
> +#define MBOX_DSTAT_REG (0x0c)
> +#define MBOX_MODE_REG (0x10)
> +#define MBOX_IMASK_REG (0x14)
> +#define MBOX_ICLR_REG (0x18)
> +#define MBOX_SEND_REG (0x1c)
> +#define MBOX_DATA_REG (0x20)
> +
> +#define MBOX_IPC_LOCK_REG (0xa00)
> +#define MBOX_IPC_UNLOCK (0x1acce551)
> +
> +#define MBOX_AUTOMATIC_ACK (1)
> +
Please, no brackets around numbers.
> +#define MBOX_STATE_IDLE BIT(4)
> +#define MBOX_STATE_ACK BIT(7)
> +
> +#define MBOX_MSG_LEN 8
> +
> +/**
> + * Hi3660 mailbox channel device data
> + *
> + * A channel can be used for TX or RX, it can trigger remote
> + * processor interrupt to notify remote processor and can receive
> + * interrupt if has incoming message.
> + *
> + * @dst_irq: Interrupt vector for remote processor
> + * @ack_irq: Interrupt vector for local processor
> + */
> +struct hi3660_mbox_dev {
>
Better than dev, maybe hi3660_chan_info ?
> + unsigned int dst_irq;
> + unsigned int ack_irq;
> +};
> +
> +/**
> + * Hi3660 mailbox controller data
> + *
> + * Mailbox controller includes 32 channels and can allocate
> + * channel for message transferring.
> + *
> + * @dev: Device to which it is attached
> + * @base: Base address of the register mapping region
> + * @chan: Representation of channels in mailbox controller
> + * @mdev: Representation of channel device data
> + * @controller: Representation of a communication channel controller
> + */
> +struct hi3660_mbox {
> + struct device *dev;
> + void __iomem *base;
> + struct mbox_chan chan[MBOX_CHAN_MAX];
> + struct hi3660_mbox_dev mdev[MBOX_CHAN_MAX];
>
Maybe mchan, instead of mdev.
> + struct mbox_controller controller;
> +};
> +
> +static inline struct hi3660_mbox *to_hi3660_mbox(struct mbox_controller *mbox)
> +{
> + return container_of(mbox, struct hi3660_mbox, controller);
> +}
> +
> +static int hi3660_mbox_check_state(struct mbox_chan *chan)
> +{
> + unsigned long ch = (unsigned long)chan->con_priv;
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> + void __iomem *base = MBOX_BASE(mbox, ch);
> + unsigned long val;
> + unsigned int state, ret;
> +
> + /* Mailbox is idle so directly bail out */
> + state = readl_relaxed(base + MBOX_MODE_REG);
> + if (state & MBOX_STATE_IDLE)
> + return 0;
> +
> + /* Wait for acknowledge from remote */
> + ret = readx_poll_timeout_atomic(readl_relaxed, base + MBOX_MODE_REG,
> + val, (val & MBOX_STATE_ACK), 1000, 300000);
> + if (ret) {
> + dev_err(mbox->dev, "%s: timeout for receiving ack\n", __func__);
> + return ret;
> + }
> +
> + /* Ensure channel is released */
> + writel_relaxed(0xffffffff, base + MBOX_IMASK_REG);
> + writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> +
> + return 0;
> +}
> +
> +static int hi3660_mbox_unlock(struct mbox_chan *chan)
> +{
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + unsigned int val, retry = 3;
> +
> + do {
> + writel_relaxed(MBOX_IPC_UNLOCK, mbox->base + MBOX_IPC_LOCK_REG);
> +
> + val = readl_relaxed(mbox->base + MBOX_IPC_LOCK_REG);
> + if (!val)
> + break;
> +
> + udelay(10);
> + } while (retry--);
> +
> + return (!val) ? 0 : -ETIMEDOUT;
> +}
> +
> +static int hi3660_mbox_acquire_channel(struct mbox_chan *chan)
> +{
> + unsigned long ch = (unsigned long)chan->con_priv;
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> + void __iomem *base = MBOX_BASE(mbox, ch);
> + unsigned int val, retry;
> +
> + for (retry = 10; retry; retry--) {
> + /* Check if channel has been acquired */
> + if (readl_relaxed(base + MBOX_MODE_REG) & MBOX_STATE_IDLE) {
> + writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> + val = readl_relaxed(base + MBOX_SRC_REG);
> + if (val & BIT(mdev->ack_irq))
> + break;
> + }
> + }
> +
> + return retry ? 0 : -ETIMEDOUT;
> +}
> +
> +static int hi3660_mbox_send(struct mbox_chan *chan, u32 *msg)
> +{
> + unsigned long ch = (unsigned long)chan->con_priv;
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> + void __iomem *base = MBOX_BASE(mbox, ch);
> + unsigned int i;
> +
> + /* Clear mask for destination interrupt */
> + writel_relaxed(~BIT(mdev->dst_irq), base + MBOX_IMASK_REG);
> +
> + /* Config destination for interrupt vector */
> + writel_relaxed(BIT(mdev->dst_irq), base + MBOX_DST_REG);
> +
> + /* Automatic acknowledge mode */
> + writel_relaxed(MBOX_AUTOMATIC_ACK, base + MBOX_MODE_REG);
> +
> + /* Fill message data */
> + for (i = 0; i < MBOX_MSG_LEN; i++)
> + writel_relaxed(msg[i], base + MBOX_DATA_REG + i * 4);
> +
> + /* Trigger data transferring */
> + writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SEND_REG);
> + return 0;
> +}
> +
> +static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)
> +{
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + int err;
> +
> + err = hi3660_mbox_check_state(chan);
> + if (err) {
> + dev_err(mbox->dev, "checking state failed\n");
> + return err;
> + }
> +
> + err = hi3660_mbox_unlock(chan);
> + if (err) {
> + dev_err(mbox->dev, "unlocking mailbox failed\n");
> + return err;
> + }
> +
> + err = hi3660_mbox_acquire_channel(chan);
> + if (err) {
> + dev_err(mbox->dev, "acquiring channel failed\n");
> + return err;
> + }
> +
Please remember .send_data() is called with irqs disabled spinlock
All these polling functions hurt. Please move the above three checks
in last_tx_done()
Thanks
--
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
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