* [PATCH] of: Fix typo in kerneldoc
From: Thierry Reding @ 2019-07-26 10:17 UTC (permalink / raw)
To: Rob Herring, Frank Rowand; +Cc: devicetree, linux-kernel
From: Thierry Reding <treding@nvidia.com>
"Findfrom" is not a word. Replace the function synopsis by something
that makes sense.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
include/linux/of.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/of.h b/include/linux/of.h
index 0cf857012f11..844f89e1b039 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1164,7 +1164,7 @@ static inline int of_property_read_string_index(const struct device_node *np,
}
/**
- * of_property_read_bool - Findfrom a property
+ * of_property_read_bool - Find a property
* @np: device node from which the property value is to be read.
* @propname: name of the property to be searched.
*
--
2.22.0
^ permalink raw reply related
* Re: [PATCH 3/3] drm/bridge: Add NWL MIPI DSI host controller support
From: Sam Ravnborg @ 2019-07-26 10:08 UTC (permalink / raw)
To: Guido Günther
Cc: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Lee Jones, dri-devel, devicetree,
linux-arm-kernel, linux-kernel, Robert Chiras
In-Reply-To: <3158f4f8c97c21f98c394e5631d74bc60d796522.1563983037.git.agx@sigxcpu.org>
Hi Guido.
Following some trivial comments.
As for the overall design I already commented on that in the binding.
(bridge versus display controller)
That it can work on top of mxsfb is a good indication that it is a
bridge but I just do not see the full picture.
In general the code looked clean and neat.
On Wed, Jul 24, 2019 at 05:52:26PM +0200, Guido Günther wrote:
> This adds initial support for the NWL MIPI DSI Host controller found on
> i.MX8 SoCs.
>
> It adds support for the i.MX8MQ but the same IP can be found on
> e.g. the i.MX8QXP.
>
> It has been tested on the Librem 5 devkit using mxsfb.
Looking at mxsfb I wonder hw this was done, as there seems to be no
bridge support in mxsfb. Using a patched version of mxsfb?
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 4934fcf5a6f8..904a9eb3a20a 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -16,4 +16,5 @@ obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
> obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
> +obj-y += imx-nwl/
obj-$(ONFIG_DRM_IMX_NWL_DSI) += imx-nwl/?
So we do not visit the directory unless required.
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/imx-nwl/Makefile
> @@ -0,0 +1,2 @@
> +imx-nwl-objs := nwl-drv.o nwl-dsi.o
The preferred syntax is
imx-nwl-y := nwl-drv.o nwl-dsi.o
See for example Makefile for mxsfb.
Consider to introduce
header-test-y += nwl-drv.h nwl-dsi.h
So we at build time check that the headers are self-contained.
(they include what they need).
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_print.h>
> +#include <drm/drm_probe_helper.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/irq.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mfd/syscon/imx8mq-iomuxc-gpr.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
> +#include <linux/regmap.h>
> +#include <linux/sys_soc.h>
> +#include <video/videomode.h>
> +
> +#include "nwl-drv.h"
> +#include "nwl-dsi.h"
The most typical order of include files are:
#include <linux/*>
#include <video/*>
#include <drm/*>
#include ""
With the empty lines in-between each block.
And sorted like is already done here.
This in general for all the files for this driver.
> +
> +static bool
> +imx_nwl_dsi_bridge_mode_fixup(struct drm_bridge *bridge,
> + const struct drm_display_mode *mode,
> + struct drm_display_mode *adjusted_mode)
> +{
> + struct imx_nwl_dsi *dsi = bridge_to_dsi(bridge);
> + struct device *dev = dsi->dev;
> + union phy_configure_opts new_cfg;
> + unsigned long phy_ref_rate;
> + int ret;
> +
> + ret = nwl_dsi_get_dphy_params(dsi, adjusted_mode, &new_cfg);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * If hs clock is unchanged, we're all good - all parameters are
> + * derived from it atm.
> + */
> + if (new_cfg.mipi_dphy.hs_clk_rate == dsi->phy_cfg.mipi_dphy.hs_clk_rate)
> + return true;
> +
> + phy_ref_rate = clk_get_rate(dsi->phy_ref_clk);
> + DRM_DEV_DEBUG_DRIVER(dev, "PHY at ref rate: %lu\n", phy_ref_rate);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dsi->dev,
> + "Cannot setup PHY for mode: %ux%u @%d Hz\n",
> + adjusted_mode->hdisplay, adjusted_mode->vdisplay,
> + adjusted_mode->clock);
> + DRM_DEV_ERROR(dsi->dev, "PHY ref clk: %lu, bit clk: %lu\n",
> + phy_ref_rate, new_cfg.mipi_dphy.hs_clk_rate);
> + } else {
> + /* Save the new desired phy config */
> + memcpy(&dsi->phy_cfg, &new_cfg, sizeof(new_cfg));
> + }
> +
> + /* LCDIF + NWL needs active high sync */
> + adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC);
> + adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC);
> +
> + drm_display_mode_to_videomode(adjusted_mode, &dsi->vm);
Hmm, the videomode is just another representation of data already
included in display_mode.
And, as a personal itch, I consider videomode as something that belongs
in the old fb drivers, and not drm drivers. But that may be me only.
Sam
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: vendor-prefixes: Add Admatec AG
From: Schrempf Frieder @ 2019-07-26 10:07 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAJKOXPdbBXEy0zzjZ1ytts0y5STQ5x9xQVBmU1vn46tmu8uCGg@mail.gmail.com>
On 26.07.19 12:02, Krzysztof Kozlowski wrote:
> On Fri, 26 Jul 2019 at 11:48, Schrempf Frieder
> <frieder.schrempf@kontron.de> wrote:
>>
>> On 26.07.19 08:17, Krzysztof Kozlowski wrote:
>>> Add vendor prefix for Admatec AG.
>>
>> We get the displays used with the Kontron eval kits from "admatec GmbH"
>> in Hamburg, not "Admatec AG" in Switzerland. I think we have to
>> differentiate here.
>>
>> I will review patch 2/2 soon...
>
> What a coincidence... they have so similar portfolio. After looking at
> vendor prefixes that would be the first duplication of name.
I have no idea, whether they are related somehow or have a common history...
>
> To avoid conflict, how about: "admatecde"?
Would be ok, I guess.
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: leds: document new "power-supply" property
From: Daniel Thompson @ 2019-07-26 10:06 UTC (permalink / raw)
To: Jean-Jacques Hiblot
Cc: Rob Herring, jacek.anaszewski, pavel, mark.rutland, dmurphy,
linux-leds, linux-kernel, devicetree
In-Reply-To: <753b2c8d-e8fc-ec6e-f372-a84d4452fd33@ti.com>
On Thu, Jul 25, 2019 at 01:08:46PM +0200, Jean-Jacques Hiblot wrote:
> Hi Rob,
>
> On 24/07/2019 18:47, Rob Herring wrote:
> > On Mon, Jul 08, 2019 at 12:35:47PM +0200, Jean-Jacques Hiblot wrote:
> > > Most of the LEDs are powered by a voltage/current regulator. describing in
> > > the device-tree makes it possible for the LED core to enable/disable it
> > > when needed.
> > >
> > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> > > ---
> > > Documentation/devicetree/bindings/leds/common.txt | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
> > > index 70876ac11367..e093a2b7eb90 100644
> > > --- a/Documentation/devicetree/bindings/leds/common.txt
> > > +++ b/Documentation/devicetree/bindings/leds/common.txt
> > > @@ -61,6 +61,11 @@ Optional properties for child nodes:
> > > - panic-indicator : This property specifies that the LED should be used,
> > > if at all possible, as a panic indicator.
> > > +- power-supply : A voltage/current regulator used to to power the LED. When a
> > > + LED is turned off, the LED core disable its regulator. The
> > > + same regulator can power many LED (or other) devices. It is
> > > + turned off only when all of its users disabled it.
> > Not sure this should be common. It wouldn't apply to cases where we have
> > an LED controller parent nor gpio and pwm LEDs and those are most cases.
>
> It does make sense for GPIO and PWM bindings if the anode of LED is tied to
> a regulated voltage and the cathod to the control line.
>
> The same is true for a certain class of true LED controller that do not
> deliver power but act like current sinks.
>
> JJ
>
> >
> > Perhaps what makes sense here is an regulator-led binding.
You didn't comment on this alternative... and I confess I'm not quite
sure what Rob means by a regulator-led binding so I can't really comment
either.
Rob, is there any analogous example for a regulator-<something-else> binding
to compare with?
Daniel.
> >
> > > +
> > > - trigger-sources : List of devices which should be used as a source triggering
> > > this LED activity. Some LEDs can be related to a specific
> > > device and should somehow indicate its state. E.g. USB 2.0
> > > --
> > > 2.17.1
> > >
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: vendor-prefixes: Add Admatec AG
From: Krzysztof Kozlowski @ 2019-07-26 10:02 UTC (permalink / raw)
To: Schrempf Frieder
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <963ba555-dde0-9c3c-1e15-740ca200853f@kontron.de>
On Fri, 26 Jul 2019 at 11:48, Schrempf Frieder
<frieder.schrempf@kontron.de> wrote:
>
> On 26.07.19 08:17, Krzysztof Kozlowski wrote:
> > Add vendor prefix for Admatec AG.
>
> We get the displays used with the Kontron eval kits from "admatec GmbH"
> in Hamburg, not "Admatec AG" in Switzerland. I think we have to
> differentiate here.
>
> I will review patch 2/2 soon...
What a coincidence... they have so similar portfolio. After looking at
vendor prefixes that would be the first duplication of name.
To avoid conflict, how about: "admatecde"?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2] arm64: dts: fast models: Remove clcd's max-memory-bandwidth
From: Sudeep Holla @ 2019-07-26 9:50 UTC (permalink / raw)
To: Kevin Brodsky
Cc: devicetree, Pawel Moll, Ruben Ayrapetyan, Linus Walleij,
Liviu Dudau, dri-devel, Sudeep Holla, Robin Murphy, Brian Starkey,
linux-arm-kernel
In-Reply-To: <20190725171215.71801-1-kevin.brodsky@arm.com>
On Thu, Jul 25, 2019 at 06:12:14PM +0100, Kevin Brodsky wrote:
> It is unclear why max-memory-bandwidth should be set for CLCD on the
> fast model. Removing that property allows allocating and using 32bpp
> buffers, which may be desirable on certain platforms such as
> Android.
>
> Reported-by: Ruben Ayrapetyan <ruben.ayrapetyan@arm.com>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
>
> Changes in v2:
> - Remove the attribute completely instead of increasing its value. It is
> optional and there is no clear reason why it should be set at all.
>
> arch/arm64/boot/dts/arm/fvp-base-revc.dts | 8 --------
> arch/arm64/boot/dts/arm/rtsm_ve-motherboard.dtsi | 2 --
I was about to ask about the above file in v1, and then saw v2.
I will take this for v5.4. Thanks
--
Regards,
Sudeep
^ permalink raw reply
* RE: [v4 1/6] dt-bindings: media: Document bindings for DW MIPI CSI-2 Host
From: Luis de Oliveira @ 2019-07-26 9:50 UTC (permalink / raw)
To: Sakari Ailus, Luis de Oliveira
Cc: mchehab@kernel.org, davem@davemloft.net,
gregkh@linuxfoundation.org, Jonathan.Cameron@huawei.com,
robh@kernel.org, nicolas.ferre@microchip.com,
paulmck@linux.ibm.com, mark.rutland@arm.com, kishon@ti.com,
devicetree@vger.kernel.org, linux-media@vger.kernel.org,
linux-kernel@vger.kernel.org, Joao.Pinto@synopsys.com
In-Reply-To: <20190725200211.GK1263@valkosipuli.retiisi.org.uk>
Hi Sakari,
Thank you for the review, my answers inline.
> From: Sakari Ailus <sakari.ailus@iki.fi>
> Date: Thu, Jul 25, 2019 at 21:02:11
>
> Hi Luis,
>
> On Wed, Jul 10, 2019 at 10:20:55AM +0000, Luis de Oliveira wrote:
> > Hi Sakari,
> >
> > From: Sakari Ailus <sakari.ailus@iki.fi>
> > Date: Tue, Jul 09, 2019 at 19:25:00
> >
> > > Hi Luis,
> > >
> > > On Mon, Jul 08, 2019 at 03:21:50PM +0000, Luis de Oliveira wrote:
> > > > Hi Sakari,
> > > >
> > > > Thank you for your feedback.
> > > > I have my comments inline.
> > > >
> > > > From: Sakari Ailus <sakari.ailus@iki.fi>
> > > > Date: Fri, Jun 28, 2019 at 15:13:26
> > > >
> > > > > Hi Luis,
> > > > >
> > > > > Thank you for the patchset.
> > > > >
> > > > > On Tue, Jun 11, 2019 at 09:20:50PM +0200, Luis Oliveira wrote:
> > > > > > From: Luis Oliveira <lolivei@synopsys.com>
> > > > > >
> > > > > > Add bindings for Synopsys DesignWare MIPI CSI-2 host.
> > > > > >
> > > > > > Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
> > > > > > ---
> > > > > > Changelog
> > > > > > v3-v4
> > > > > > - remove "plat" from the block name @rob @laurent
> > > > > > - remove "phy-names" when single-entry @rob
> > > > > > - remove "snps,output-type" -> went to the driver config @laurent
> > > > > >
> > > > > > .../devicetree/bindings/media/snps,dw-csi.txt | 41 ++++++++++++++++++++++
> > > > > > 1 file changed, 41 insertions(+)
> > > > > > create mode 100644 Documentation/devicetree/bindings/media/snps,dw-csi.txt
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/media/snps,dw-csi.txt b/Documentation/devicetree/bindings/media/snps,dw-csi.txt
> > > > > > new file mode 100644
> > > > > > index 0000000..613b7f9
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/media/snps,dw-csi.txt
> > > > > > @@ -0,0 +1,41 @@
> > > > > > +Synopsys DesignWare CSI-2 Host controller
> > > > > > +
> > > > > > +Description
> > > > > > +-----------
> > > > > > +
> > > > > > +This HW block is used to receive image coming from an MIPI CSI-2 compatible
> > > > > > +camera.
> > > > > > +
> > > > > > +Required properties:
> > > > > > +- compatible : shall be "snps,dw-csi"
> > > > > > +- reg : physical base address and size of the device memory
> > > > > > + mapped registers;
> > > > > > +- interrupts : DW CSI-2 Host interrupts
> > > > > > +- phys : List of one PHY specifier (as defined in
> > > > > > + Documentation/devicetree/bindings/phy/phy-bindings.txt).
> > > > > > + This PHY is a MIPI DPHY working in RX mode.
> > > > > > +- resets : Reference to a reset controller (optional)
> > > > > > +
> > > > > > +The per-board settings:
> > > > > > + - port sub-node describing a single endpoint connected to the camera as
> > > > > > + described in video-interfaces.txt[1].
> > > > >
> > > > > Which endpoint properties in video-interfaces.txt are relevant for the
> > > > > hardware? Which values may they have?
> > > > >
> > > >
> > > > Currently I'm using only two properties "data-lanes" and "bus-width", but
> > > > I have plans to add blanking info also.
> > > > I will add more info.
> > >
> > > Isn't blanking defined by what the transmitter seneds? Or do you have
> > > hardware limitations on the receiver side?
> > >
> >
> > When we use this IP in prototyping we configure blanking at the receiver
> > side.
> > Some cameras don't have blanking configuration capabilities so we
> > configure it on the RX side.
>
> I haven't come across a CSI-2 connected camera without some kind of
> blanking configuration capabilities. Even if there was one, you couldn't
> configure blanking from the receiver side.
>
> Please document that the data-lanes property is required, and which values
> are possible.
>
Ok, I will add the data-lanes property to the Documentation.
> >
> > > I've only heard of one such case before, and it was a very old parallel
> > > receiver.
> > >
> > > If you have a CSI-2 receiver, bus-width isn't relevant --- it's for paralle
> > > interfaces only. Please add data-lanes to required endpoint properties.
> > >
> >
> > I used bus-width property in the Synopsys IPI (Image Pixel Interface)
> > that enables direct video stream access.
> > This interface is an output that can be 16-bit or 48-bit, that's why I
> > used bus-width property.
>
> Does this device write the image data to system memory, or is it another
> device? If there's another one, then you should probably have another port
> to describe that connection.
>
Yes, it is another. I can add that connection port also.
> --
> Kind regards,
>
> Sakari Ailus
Thank you,
Luis
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: vendor-prefixes: Add Admatec AG
From: Schrempf Frieder @ 2019-07-26 9:48 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Mark Rutland, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190726061705.14764-1-krzk@kernel.org>
On 26.07.19 08:17, Krzysztof Kozlowski wrote:
> Add vendor prefix for Admatec AG.
We get the displays used with the Kontron eval kits from "admatec GmbH"
in Hamburg, not "Admatec AG" in Switzerland. I think we have to
differentiate here.
I will review patch 2/2 soon...
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> ---
>
> Changes since v1:
> New patch
> ---
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 6992bbbbffab..94c816f74209 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -43,6 +43,8 @@ patternProperties:
> description: AD Holdings Plc.
> "^adi,.*":
> description: Analog Devices, Inc.
> + "^admatec,.*":
> + description: Admatec AG
> "^advantech,.*":
> description: Advantech Corporation
> "^aeroflexgaisler,.*":
>
^ permalink raw reply
* Re: [PATCH V3 1/3] mmc: mmci: fix read status for busy detect
From: Ludovic BARRE @ 2019-07-26 9:41 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rob Herring, Srinivas Kandagatla, Maxime Coquelin,
Alexandre Torgue, Linux ARM, Linux Kernel Mailing List, DTML,
linux-mmc@vger.kernel.org, linux-stm32
In-Reply-To: <CAPDyKFpJPbpTnfA2cynFURyxFY_YCm7MRXw3m2nQyU+z=ZWsFA@mail.gmail.com>
hi Ulf
Thanks to your "Clarify comments ..." commit, like is closes
I resumed upstream of this series.
On 7/15/19 6:31 PM, Ulf Hansson wrote:
> On Mon, 3 Jun 2019 at 17:55, Ludovic Barre <ludovic.Barre@st.com> wrote:
>>
>> From: Ludovic Barre <ludovic.barre@st.com>
>>
>> "busy_detect_flag" is used to read & clear busy value of mmci status.
>> "busy_detect_mask" is used to manage busy irq of mmci mask.
>> So to read mmci status the busy_detect_flag must be take account.
>> if the variant does not support busy detect feature the flag is null
>> and there is no impact.
>
> By reading the changelog, it doesn't tell me the purpose of this
> change. When going forward, please work harder on your changelogs.
>
> Make sure to answer the questions, *why* is this change needed,
> *what/how* does the change do.
Ok, I will explain the differences with the legacy and the needs of
sdmmc variant about busy detection.
>
>>
>> Not need to re-read the status register in mmci_cmd_irq, the
>> status parameter can be used.
>>
>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>> ---
>> drivers/mmc/host/mmci.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index 356833a..5b5cc45 100644
>> --- a/drivers/mmc/host/mmci.c
>> +++ b/drivers/mmc/host/mmci.c
>> @@ -1240,7 +1240,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
>> */
>> if (!host->busy_status &&
>> !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) &&
>> - (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) {
>> + (status & host->variant->busy_detect_flag)) {
>
> I suggested you to do this change through some of my earlier comments,
> however I think it should be made as a stand alone change.
>
> Anyway, when looking at the details in your series, I decided to try
> to help out a bit, so I have prepared a couple of related patches for
> cleaning up and clarifying the busy detection code/comments in mmci. I
> have incorporated the above change, so let me post them asap.
>
>>
>> /* Clear the busy start IRQ */
>> writel(host->variant->busy_detect_mask,
>> @@ -1517,7 +1517,8 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
>> * to make sure that both start and end interrupts are always
>> * cleared one after the other.
>> */
>> - status &= readl(host->base + MMCIMASK0);
>> + status &= readl(host->base + MMCIMASK0) |
>> + host->variant->busy_detect_flag;
>
> As I told earlier in the review, this looks wrong to me.
>
> It means that you will add the bit for the ->busy_detect_flag to the
> status field we have just read from the MMCISTATUS register. That
> means the busy status may be set when it shouldn't.
>
>> if (host->variant->busy_detect)
>> writel(status & ~host->variant->busy_detect_mask,
>> host->base + MMCICLEAR);
>> --
>> 2.7.4
>>
>
> By looking at the other changes in the series, I assume @subject patch
> is intended to prepare for the other changes on top. But it's not
> really clear.
>
> Anyway, in that regards, the below is my observations of what seems to
> be important part, when supporting busy detection for the stm32 sdmmc
> variant (except the timeout things in patch2, which I intend to
> comment separately on).
>
> I figured, these are the involved register bits/masks:
>
> MMCISTATUS:
> MCI_STM32_BUSYD0 BIT(20)
> MCI_STM32_BUSYD0END BIT(21)
>
> MMCIMASK0:
> MCI_STM32_BUSYD0ENDMASK BIT(21)
it's exact:
MCI_STM32_BUSYD0 BIT(20): This is a hardware status flag only (inverted
value of d0 line), it does not generate an interrupt, and so no mask
bit.
MCI_STM32_BUSYD0ENDMASK BIT(21): This indicates only end of busy
following a CMD response. On busy to Not busy changes, an interrupt
is generated (if unmask) and BUSYD0END status flag is set.
status flag is cleared by writing corresponding interrupt clear bit in
MMCICLEAR.
>
> For the legacy ST variant, there is only one register bit in
> MMCISTATUS that is used for indicating busy (MCI_ST_CARDBUSY BIT(24)).
> There is no dedicated busy-end bit for the busy-end IRQ, which I
> believe is the reason to why the current code also is bit messy.
yes
>
> It seems like the stm32 sdmmc variant have a separate status bit for
> the busy-end IRQ, correct?
yes
>
> If I understand correctly by looking at patch3, you don't use the
> dedicated busy-end status bit (MCI_STM32_BUSYD0END), right? Then why
> not?
like your are clarify in previous series, the busy detection is done
in 3 steps.
if I use:
.busy_detect_flag = MCI_STM32_BUSYD0ENDMASK,
.busy_detect_mask = MCI_STM32_BUSYD0ENDMASK,
the sdmmc request will be not correctly completed, because the third
step can't be happen.
chronologies:
step1: when busyd0end change to 1
=> busyd0end interrupt is unmasked
=> busy_status = cmd_sent | respend
=> return to mmci_irq
step2: busyd0end is yet to 1
=> clear the busyd0end interrupt
=> the hardware clear busyd0end status flag on interrupt clear
=> return to mmci_irq
like MCI_STM32_BUSYD0END interrupt is generated only on change
busy to not busy, when the interrupt is cleared (status is 0)
the step 3 can't happen (no irq pending to re-enter in mmci_cmd_irq).
sdmmc can't complete the request.
If I use:
.busy_detect_flag = MCI_STM32_BUSYD0,
.busy_detect_mask = MCI_STM32_BUSYD0ENDMASK,
Like there is no MCI_STM32_BUSYD0 irq mask, the status read in mmci_irq
"status &= readl(host->base + MMCIMASK0)" can't take account the
busy_detect_flag (for sdmmc). So the step 2 can't be passed.
However we could share re-read between step 1 and step 2.
proposal:
+
+ u32 busy_val = readl(base + MMCISTATUS) &
+ host->variant->busy_detect_flag;
+
if (!host->busy_status &&
- !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) &&
- (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) {
+ !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && busy_val) {
writel(readl(base + MMCIMASK0) |
host->variant->busy_detect_mask,
@@ -1262,8 +1265,7 @@ mmci_cmd_irq(struct mmci_host *host, struct
mmc_command *cmd,
* both the start and the end interrupts needs to be cleared,
* one after the other. So, clear the busy start IRQ here.
*/
- if (host->busy_status &&
- (status & host->variant->busy_detect_flag)) {
+ if (host->busy_status && busy_val) {
what do you think about it ?
>
> Thoughts?
>
> Kind regards
> Uffe
>
Regards
Ludo
^ permalink raw reply
* [PATCH v5 7/7] remoteproc: qcom: wcss: explicitly request exclusive reset control
From: Govind Singh @ 2019-07-26 9:23 UTC (permalink / raw)
To: bjorn.andersson, linux-remoteproc, sboyd, linux-clk,
linux-arm-msm
Cc: andy.gross, linux-soc, devicetree, Govind Singh
In-Reply-To: <20190726092332.25202-1-govinds@codeaurora.org>
Use request exclusive reset control for wcss reset controls.
Signed-off-by: Govind Singh <govinds@codeaurora.org>
---
drivers/remoteproc/qcom_q6v5_wcss.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index c2e75f5b5737..a2671a19f2d0 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -781,21 +781,21 @@ static int q6v5_wcss_init_reset(struct q6v5_wcss *wcss,
struct device *dev = wcss->dev;
if (desc->aon_reset_required) {
- wcss->wcss_aon_reset = devm_reset_control_get(dev, "wcss_aon_reset");;
+ wcss->wcss_aon_reset = devm_reset_control_get_exclusive(dev, "wcss_aon_reset");;
if (IS_ERR(wcss->wcss_aon_reset)) {
dev_err(wcss->dev, "fail to acquire wcss_aon_reset\n");
return PTR_ERR(wcss->wcss_aon_reset);
}
}
- wcss->wcss_reset = devm_reset_control_get(dev, "wcss_reset");
+ wcss->wcss_reset = devm_reset_control_get_exclusive(dev, "wcss_reset");
if (IS_ERR(wcss->wcss_reset)) {
dev_err(wcss->dev, "unable to acquire wcss_reset\n");
return PTR_ERR(wcss->wcss_reset);
}
if (desc->wcss_q6_reset_required) {
- wcss->wcss_q6_reset = devm_reset_control_get(dev, "wcss_q6_reset");
+ wcss->wcss_q6_reset = devm_reset_control_get_exclusive(dev, "wcss_q6_reset");
if (IS_ERR(wcss->wcss_q6_reset)) {
dev_err(wcss->dev, "unable to acquire wcss_q6_reset\n");
return PTR_ERR(wcss->wcss_q6_reset);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH v5 6/7] remoteproc: qcom: wcss: Add non pas wcss Q6 support for QCS404
From: Govind Singh @ 2019-07-26 9:23 UTC (permalink / raw)
To: bjorn.andersson, linux-remoteproc, sboyd, linux-clk,
linux-arm-msm
Cc: andy.gross, linux-soc, devicetree, Govind Singh
In-Reply-To: <20190726092332.25202-1-govinds@codeaurora.org>
Add non PAS WCSS remoteproc driver support for QCS404 SOC.
Add WCSS q6 bootup and shutdown sequence handled from
Application Processor SubSystem(APSS).
Signed-off-by: Govind Singh <govinds@codeaurora.org>
---
drivers/remoteproc/qcom_q6v5_wcss.c | 570 ++++++++++++++++++++++++++--
1 file changed, 532 insertions(+), 38 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index 84accf1755e9..c2e75f5b5737 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -4,14 +4,19 @@
* Copyright (C) 2014 Sony Mobile Communications AB
* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved.
*/
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
#include <linux/iopoll.h>
#include <linux/kernel.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
+#include <linux/of_address.h>
#include <linux/of_device.h>
#include <linux/of_reserved_mem.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <linux/reset.h>
#include <linux/soc/qcom/mdt_loader.h>
#include "qcom_common.h"
@@ -24,6 +29,9 @@
#define Q6SS_GFMUX_CTL_REG 0x020
#define Q6SS_PWR_CTL_REG 0x030
#define Q6SS_MEM_PWR_CTL 0x0B0
+#define Q6SS_STRAP_ACC 0x110
+#define Q6SS_CGC_OVERRIDE 0x034
+#define Q6SS_BCR_REG 0x6000
/* AXI Halt Register Offsets */
#define AXI_HALTREQ_REG 0x0
@@ -37,14 +45,19 @@
#define Q6SS_CORE_ARES BIT(1)
#define Q6SS_BUS_ARES_ENABLE BIT(2)
+/* Q6SS_BRC_RESET */
+#define Q6SS_BRC_BLK_ARES BIT(0)
+
/* Q6SS_GFMUX_CTL */
#define Q6SS_CLK_ENABLE BIT(1)
+#define Q6SS_SWITCH_CLK_SRC BIT(8)
/* Q6SS_PWR_CTL */
#define Q6SS_L2DATA_STBY_N BIT(18)
#define Q6SS_SLP_RET_N BIT(19)
#define Q6SS_CLAMP_IO BIT(20)
#define QDSS_BHS_ON BIT(21)
+#define QDSS_Q6_MEMORIES GENMASK(15, 0)
/* Q6SS parameters */
#define Q6SS_LDO_BYP BIT(25)
@@ -53,6 +66,7 @@
#define Q6SS_CLAMP_QMC_MEM BIT(22)
#define HALT_CHECK_MAX_LOOPS 200
#define Q6SS_XO_CBCR GENMASK(5, 3)
+#define Q6SS_SLEEP_CBCR GENMASK(5, 2)
/* Q6SS config/status registers */
#define TCSR_GLOBAL_CFG0 0x0
@@ -71,9 +85,23 @@
#define TCSR_WCSS_CLK_MASK 0x1F
#define TCSR_WCSS_CLK_ENABLE 0x14
+#define MAX_HALT_REG 3
+enum {
+ WCSS_IPQ8074,
+ WCSS_QCS404,
+};
+
struct wcss_data {
const char *firmware_name;
int crash_reason_smem;
+ u32 version;
+ bool aon_reset_required;
+ bool wcss_q6_reset_required;
+ const char *ssr_name;
+ const char *sysmon_name;
+ int ssctl_id;
+ const struct rproc_ops *ops;
+ bool requires_force_stop;
};
struct q6v5_wcss {
@@ -87,9 +115,29 @@ struct q6v5_wcss {
u32 halt_wcss;
u32 halt_nc;
+ struct clk *xo;
+ struct clk *ahbfabric_cbcr_clk;
+ struct clk *gcc_abhs_cbcr;
+ struct clk *gcc_axim_cbcr;
+ struct clk *lcc_csr_cbcr;
+ struct clk *ahbs_cbcr;
+ struct clk *tcm_slave_cbcr;
+ struct clk *qdsp6ss_abhm_cbcr;
+ struct clk *qdsp6ss_sleep_cbcr;
+ struct clk *qdsp6ss_axim_cbcr;
+ struct clk *qdsp6ss_xo_cbcr;
+ struct clk *qdsp6ss_core_gfmux;
+ struct clk *lcc_bcr_sleep;
+ struct regulator *cx_supply;
+
+ struct qcom_rproc_glink glink_subdev;
+ struct qcom_rproc_ssr ssr_subdev;
+ struct qcom_sysmon *sysmon;
+
struct reset_control *wcss_aon_reset;
struct reset_control *wcss_reset;
struct reset_control *wcss_q6_reset;
+ struct reset_control *wcss_q6_bcr_reset;
struct qcom_q6v5 q6v5;
@@ -99,6 +147,8 @@ struct q6v5_wcss {
size_t mem_size;
int crash_reason_smem;
+ u32 version;
+ bool requires_force_stop;
};
static int q6v5_wcss_reset(struct q6v5_wcss *wcss)
@@ -241,6 +291,207 @@ static int q6v5_wcss_start(struct rproc *rproc)
return ret;
}
+static int q6v5_wcss_qcs404_power_on(struct q6v5_wcss *wcss)
+{
+ unsigned long val;
+ int ret, idx;
+
+ /* Toggle the restart */
+ reset_control_assert(wcss->wcss_reset);
+ usleep_range(200, 300);
+ reset_control_deassert(wcss->wcss_reset);
+ usleep_range(200, 300);
+
+ /* Enable GCC_WDSP_Q6SS_AHBS_CBCR clock */
+ ret = clk_prepare_enable(wcss->gcc_abhs_cbcr);
+ if (ret)
+ return ret;
+
+ /* Remove reset to the WCNSS QDSP6SS */
+ reset_control_deassert(wcss->wcss_q6_bcr_reset);
+
+ /* Enable Q6SSTOP_AHBFABRIC_CBCR clock */
+ ret = clk_prepare_enable(wcss->ahbfabric_cbcr_clk);
+ if (ret)
+ goto disable_gcc_abhs_cbcr_clk;
+
+ /* Enable the LCCCSR CBC clock, Q6SSTOP_Q6SSTOP_LCC_CSR_CBCR clock */
+ ret = clk_prepare_enable(wcss->lcc_csr_cbcr);
+ if (ret)
+ goto disable_ahbfabric_cbcr_clk;
+
+ /* Enable the Q6AHBS CBC, Q6SSTOP_Q6SS_AHBS_CBCR clock */
+ ret = clk_prepare_enable(wcss->ahbs_cbcr);
+ if (ret)
+ goto disable_csr_cbcr_clk;
+
+ /* Enable the TCM slave CBC, Q6SSTOP_Q6SS_TCM_SLAVE_CBCR clock */
+ ret = clk_prepare_enable(wcss->tcm_slave_cbcr);
+ if (ret)
+ goto disable_ahbs_cbcr_clk;
+
+ /* Enable the Q6SS AHB master CBC, Q6SSTOP_Q6SS_AHBM_CBCR clock */
+ ret = clk_prepare_enable(wcss->qdsp6ss_abhm_cbcr);
+ if (ret)
+ goto disable_tcm_slave_cbcr_clk;
+
+ /* Enable the Q6SS AXI master CBC, Q6SSTOP_Q6SS_AXIM_CBCR clock */
+ ret = clk_prepare_enable(wcss->qdsp6ss_axim_cbcr);
+ if (ret)
+ goto disable_abhm_cbcr_clk;
+
+ /* Enable the Q6SS XO CBC */
+ val = readl(wcss->reg_base + Q6SS_XO_CBCR);
+ val |= BIT(0);
+ writel(val, wcss->reg_base + Q6SS_XO_CBCR);
+ /* Read CLKOFF bit to go low indicating CLK is enabled */
+ ret = readl_poll_timeout(wcss->reg_base + Q6SS_XO_CBCR,
+ val, !(val & BIT(31)), 1,
+ HALT_CHECK_MAX_LOOPS);
+ if (ret) {
+ dev_err(wcss->dev,
+ "xo cbcr enabling timed out (rc:%d)\n", ret);
+ return ret;
+ }
+
+ writel(0, wcss->reg_base + Q6SS_CGC_OVERRIDE);
+
+ /* Enable QDSP6 sleep clock clock */
+ val = readl(wcss->reg_base + Q6SS_SLEEP_CBCR);
+ val |= BIT(0);
+ writel(val, wcss->reg_base + Q6SS_SLEEP_CBCR);
+
+ /* Enable the Enable the Q6 AXI clock, GCC_WDSP_Q6SS_AXIM_CBCR*/
+ ret = clk_prepare_enable(wcss->gcc_axim_cbcr);
+ if (ret)
+ goto disable_sleep_cbcr_clk;
+
+ /* Assert resets, stop core */
+ val = readl(wcss->reg_base + Q6SS_RESET_REG);
+ val |= Q6SS_CORE_ARES | Q6SS_BUS_ARES_ENABLE | Q6SS_STOP_CORE;
+ writel(val, wcss->reg_base + Q6SS_RESET_REG);
+
+ /* Program the QDSP6SS PWR_CTL register */
+ writel(0x01700000, wcss->reg_base + Q6SS_PWR_CTL_REG);
+
+ writel(0x03700000, wcss->reg_base + Q6SS_PWR_CTL_REG);
+
+ writel(0x03300000, wcss->reg_base + Q6SS_PWR_CTL_REG);
+
+ writel(0x033C0000, wcss->reg_base + Q6SS_PWR_CTL_REG);
+
+ /*
+ * Enable memories by turning on the QDSP6 memory foot/head switch, one
+ * bank at a time to avoid in-rush current
+ */
+ for (idx = 28; idx >= 0; idx--) {
+ writel((readl(wcss->reg_base + Q6SS_MEM_PWR_CTL) |
+ (1 << idx)), wcss->reg_base + Q6SS_MEM_PWR_CTL);
+ }
+
+ writel(0x031C0000, wcss->reg_base + Q6SS_PWR_CTL_REG);
+ writel(0x030C0000, wcss->reg_base + Q6SS_PWR_CTL_REG);
+
+ val = readl(wcss->reg_base + Q6SS_RESET_REG);
+ val &= ~Q6SS_CORE_ARES;
+ writel(val, wcss->reg_base + Q6SS_RESET_REG);
+
+ /* Enable the Q6 core clock at the GFM, Q6SSTOP_QDSP6SS_GFMUX_CTL */
+ val = readl(wcss->reg_base + Q6SS_GFMUX_CTL_REG);
+ val |= Q6SS_CLK_ENABLE | Q6SS_SWITCH_CLK_SRC;
+ writel(val, wcss->reg_base + Q6SS_GFMUX_CTL_REG);
+
+ /* Enable sleep clock branch needed for BCR circuit */
+ ret = clk_prepare_enable(wcss->lcc_bcr_sleep);
+ if (ret)
+ goto disable_core_gfmux_clk;
+
+ return 0;
+
+disable_core_gfmux_clk:
+ val = readl(wcss->reg_base + Q6SS_GFMUX_CTL_REG);
+ val &= ~(Q6SS_CLK_ENABLE | Q6SS_SWITCH_CLK_SRC);
+ writel(val, wcss->reg_base + Q6SS_GFMUX_CTL_REG);
+ clk_disable_unprepare(wcss->gcc_axim_cbcr);
+disable_sleep_cbcr_clk:
+ val = readl(wcss->reg_base + Q6SS_SLEEP_CBCR);
+ val &= ~Q6SS_CLK_ENABLE;
+ writel(val, wcss->reg_base + Q6SS_SLEEP_CBCR);
+ val = readl(wcss->reg_base + Q6SS_XO_CBCR);
+ val &= ~Q6SS_CLK_ENABLE;
+ writel(val, wcss->reg_base + Q6SS_XO_CBCR);
+ clk_disable_unprepare(wcss->qdsp6ss_axim_cbcr);
+disable_abhm_cbcr_clk:
+ clk_disable_unprepare(wcss->qdsp6ss_abhm_cbcr);
+disable_tcm_slave_cbcr_clk:
+ clk_disable_unprepare(wcss->tcm_slave_cbcr);
+disable_ahbs_cbcr_clk:
+ clk_disable_unprepare(wcss->ahbs_cbcr);
+disable_csr_cbcr_clk:
+ clk_disable_unprepare(wcss->lcc_csr_cbcr);
+disable_ahbfabric_cbcr_clk:
+ clk_disable_unprepare(wcss->ahbfabric_cbcr_clk);
+disable_gcc_abhs_cbcr_clk:
+ clk_disable_unprepare(wcss->gcc_abhs_cbcr);
+
+ return ret;
+}
+
+static inline int q6v5_wcss_qcs404_reset(struct q6v5_wcss *wcss)
+{
+ unsigned long val;
+
+ writel(0x80800000, wcss->reg_base + Q6SS_STRAP_ACC);
+
+ /* Start core execution */
+ val = readl(wcss->reg_base + Q6SS_RESET_REG);
+ val &= ~Q6SS_STOP_CORE;
+ writel(val, wcss->reg_base + Q6SS_RESET_REG);
+
+ return 0;
+}
+
+static int q6v5_qcs404_wcss_start(struct rproc *rproc)
+{
+ struct q6v5_wcss *wcss = rproc->priv;
+ int ret;
+
+ ret = clk_prepare_enable(wcss->xo);
+ if (ret)
+ return ret;
+
+ ret = regulator_enable(wcss->cx_supply);
+ if (ret)
+ goto disable_xo_clk;
+
+ qcom_q6v5_prepare(&wcss->q6v5);
+
+ ret = q6v5_wcss_qcs404_power_on(wcss);
+ if (ret) {
+ dev_err(wcss->dev, "wcss clk_enable failed\n");
+ goto disable_cx_supply;
+ }
+
+ writel(rproc->bootaddr >> 4, wcss->reg_base + Q6SS_RST_EVB);
+
+ q6v5_wcss_qcs404_reset(wcss);
+
+ ret = qcom_q6v5_wait_for_start(&wcss->q6v5, 5 * HZ);
+ if (ret == -ETIMEDOUT) {
+ dev_err(wcss->dev, "start timed out\n");
+ goto disable_cx_supply;
+ }
+
+ return 0;
+
+disable_cx_supply:
+ regulator_disable(wcss->cx_supply);
+disable_xo_clk:
+ clk_disable_unprepare(wcss->xo);
+
+ return ret;
+}
+
static void q6v5_wcss_halt_axi_port(struct q6v5_wcss *wcss,
struct regmap *halt_map,
u32 offset)
@@ -275,6 +526,70 @@ static void q6v5_wcss_halt_axi_port(struct q6v5_wcss *wcss,
regmap_write(halt_map, offset + AXI_HALTREQ_REG, 0);
}
+static int q6v5_qcs404_wcss_shutdown(struct q6v5_wcss *wcss)
+{
+ unsigned long val;
+ int ret;
+
+ q6v5_wcss_halt_axi_port(wcss, wcss->halt_map, wcss->halt_wcss);
+
+ /* assert clamps to avoid MX current inrush */
+ val = readl(wcss->reg_base + Q6SS_PWR_CTL_REG);
+ val |= (Q6SS_CLAMP_IO | Q6SS_CLAMP_WL | Q6SS_CLAMP_QMC_MEM);
+ writel(val, wcss->reg_base + Q6SS_PWR_CTL_REG);
+
+ /* Disable memories by turning off memory foot/headswitch */
+ writel((readl(wcss->reg_base + Q6SS_MEM_PWR_CTL) &
+ ~QDSS_Q6_MEMORIES),
+ wcss->reg_base + Q6SS_MEM_PWR_CTL);
+
+ /* Clear the BHS_ON bit */
+ val = readl(wcss->reg_base + Q6SS_PWR_CTL_REG);
+ val &= ~Q6SS_BHS_ON;
+ writel(val, wcss->reg_base + Q6SS_PWR_CTL_REG);
+
+ clk_disable_unprepare(wcss->ahbfabric_cbcr_clk);
+ clk_disable_unprepare(wcss->lcc_csr_cbcr);
+ clk_disable_unprepare(wcss->tcm_slave_cbcr);
+ clk_disable_unprepare(wcss->qdsp6ss_abhm_cbcr);
+ clk_disable_unprepare(wcss->qdsp6ss_axim_cbcr);
+
+ val = readl(wcss->reg_base + Q6SS_SLEEP_CBCR);
+ val &= ~BIT(0);
+ writel(val, wcss->reg_base + Q6SS_SLEEP_CBCR);
+
+ val = readl(wcss->reg_base + Q6SS_XO_CBCR);
+ val &= ~BIT(0);
+ writel(val, wcss->reg_base + Q6SS_XO_CBCR);
+
+ clk_disable_unprepare(wcss->ahbs_cbcr);
+ clk_disable_unprepare(wcss->lcc_bcr_sleep);
+
+ val = readl(wcss->reg_base + Q6SS_GFMUX_CTL_REG);
+ val &= ~(Q6SS_CLK_ENABLE | Q6SS_SWITCH_CLK_SRC);
+ writel(val, wcss->reg_base + Q6SS_GFMUX_CTL_REG);
+
+ clk_disable_unprepare(wcss->gcc_abhs_cbcr);
+
+ ret = reset_control_assert(wcss->wcss_reset);
+ if (ret) {
+ dev_err(wcss->dev, "wcss_reset failed\n");
+ return ret;
+ }
+ usleep_range(200, 300);
+
+ ret = reset_control_deassert(wcss->wcss_reset);
+ if (ret) {
+ dev_err(wcss->dev, "wcss_reset failed\n");
+ return ret;
+ }
+ usleep_range(200, 300);
+
+ clk_disable_unprepare(wcss->gcc_axim_cbcr);
+
+ return 0;
+}
+
static int q6v5_wcss_powerdown(struct q6v5_wcss *wcss)
{
int ret;
@@ -394,20 +709,28 @@ static int q6v5_wcss_stop(struct rproc *rproc)
int ret;
/* WCSS powerdown */
- ret = qcom_q6v5_request_stop(&wcss->q6v5);
- if (ret == -ETIMEDOUT) {
- dev_err(wcss->dev, "timed out on wait\n");
- return ret;
+ if (wcss->requires_force_stop) {
+ ret = qcom_q6v5_request_stop(&wcss->q6v5);
+ if (ret == -ETIMEDOUT) {
+ dev_err(wcss->dev, "timed out on wait\n");
+ return ret;
+ }
}
- ret = q6v5_wcss_powerdown(wcss);
- if (ret)
- return ret;
-
- /* Q6 Power down */
- ret = q6v5_q6_powerdown(wcss);
- if (ret)
- return ret;
+ if (wcss->version == WCSS_QCS404) {
+ ret = q6v5_qcs404_wcss_shutdown(wcss);
+ if (ret)
+ return ret;
+ } else {
+ ret = q6v5_wcss_powerdown(wcss);
+ if (ret)
+ return ret;
+
+ /* Q6 Power down */
+ ret = q6v5_q6_powerdown(wcss);
+ if (ret)
+ return ret;
+ }
qcom_q6v5_unprepare(&wcss->q6v5);
@@ -443,14 +766,26 @@ static const struct rproc_ops q6v5_wcss_ipq8074_ops = {
.get_boot_addr = rproc_elf_get_boot_addr,
};
-static int q6v5_wcss_init_reset(struct q6v5_wcss *wcss)
+static const struct rproc_ops q6v5_wcss_qcs404_ops = {
+ .start = q6v5_qcs404_wcss_start,
+ .stop = q6v5_wcss_stop,
+ .da_to_va = q6v5_wcss_da_to_va,
+ .load = q6v5_wcss_load,
+ .get_boot_addr = rproc_elf_get_boot_addr,
+ .parse_fw = qcom_register_dump_segments,
+};
+
+static int q6v5_wcss_init_reset(struct q6v5_wcss *wcss,
+ const struct wcss_data *desc)
{
struct device *dev = wcss->dev;
- wcss->wcss_aon_reset = devm_reset_control_get(dev, "wcss_aon_reset");
- if (IS_ERR(wcss->wcss_aon_reset)) {
- dev_err(wcss->dev, "unable to acquire wcss_aon_reset\n");
- return PTR_ERR(wcss->wcss_aon_reset);
+ if (desc->aon_reset_required) {
+ wcss->wcss_aon_reset = devm_reset_control_get(dev, "wcss_aon_reset");;
+ if (IS_ERR(wcss->wcss_aon_reset)) {
+ dev_err(wcss->dev, "fail to acquire wcss_aon_reset\n");
+ return PTR_ERR(wcss->wcss_aon_reset);
+ }
}
wcss->wcss_reset = devm_reset_control_get(dev, "wcss_reset");
@@ -459,10 +794,18 @@ static int q6v5_wcss_init_reset(struct q6v5_wcss *wcss)
return PTR_ERR(wcss->wcss_reset);
}
- wcss->wcss_q6_reset = devm_reset_control_get(dev, "wcss_q6_reset");
- if (IS_ERR(wcss->wcss_q6_reset)) {
- dev_err(wcss->dev, "unable to acquire wcss_q6_reset\n");
- return PTR_ERR(wcss->wcss_q6_reset);
+ if (desc->wcss_q6_reset_required) {
+ wcss->wcss_q6_reset = devm_reset_control_get(dev, "wcss_q6_reset");
+ if (IS_ERR(wcss->wcss_q6_reset)) {
+ dev_err(wcss->dev, "unable to acquire wcss_q6_reset\n");
+ return PTR_ERR(wcss->wcss_q6_reset);
+ }
+ }
+
+ wcss->wcss_q6_bcr_reset = devm_reset_control_get_exclusive(dev, "wcss_q6_bcr_reset");
+ if (IS_ERR(wcss->wcss_q6_bcr_reset)) {
+ dev_err(wcss->dev, "unable to acquire wcss_q6_bcr_reset\n");
+ return PTR_ERR(wcss->wcss_q6_bcr_reset);
}
return 0;
@@ -471,35 +814,48 @@ static int q6v5_wcss_init_reset(struct q6v5_wcss *wcss)
static int q6v5_wcss_init_mmio(struct q6v5_wcss *wcss,
struct platform_device *pdev)
{
- struct of_phandle_args args;
+ unsigned int halt_reg[MAX_HALT_REG] = {0};
+ struct device_node *syscon;
struct resource *res;
int ret;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qdsp6");
- wcss->reg_base = devm_ioremap_resource(&pdev->dev, res);
+ wcss->reg_base = devm_ioremap(&pdev->dev, res->start,
+ resource_size(res));
if (IS_ERR(wcss->reg_base))
return PTR_ERR(wcss->reg_base);
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rmb");
- wcss->rmb_base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(wcss->rmb_base))
- return PTR_ERR(wcss->rmb_base);
+ if (wcss->version == WCSS_IPQ8074) {
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rmb");
+ wcss->rmb_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(wcss->rmb_base))
+ return PTR_ERR(wcss->rmb_base);
+ }
- ret = of_parse_phandle_with_fixed_args(pdev->dev.of_node,
- "qcom,halt-regs", 3, 0, &args);
- if (ret < 0) {
+ syscon = of_parse_phandle(pdev->dev.of_node,
+ "qcom,halt-regs", 0);
+ if (!syscon) {
dev_err(&pdev->dev, "failed to parse qcom,halt-regs\n");
return -EINVAL;
}
- wcss->halt_map = syscon_node_to_regmap(args.np);
- of_node_put(args.np);
+ wcss->halt_map = syscon_node_to_regmap(syscon);
+ of_node_put(syscon);
if (IS_ERR(wcss->halt_map))
return PTR_ERR(wcss->halt_map);
- wcss->halt_q6 = args.args[0];
- wcss->halt_wcss = args.args[1];
- wcss->halt_nc = args.args[2];
+ ret = of_property_read_variable_u32_array(pdev->dev.of_node,
+ "qcom,halt-regs",
+ halt_reg, 0,
+ MAX_HALT_REG);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to parse qcom,halt-regs\n");
+ return -EINVAL;
+ }
+
+ wcss->halt_q6 = halt_reg[0];
+ wcss->halt_wcss = halt_reg[1];
+ wcss->halt_nc = halt_reg[2];
return 0;
}
@@ -533,6 +889,107 @@ static int q6v5_alloc_memory_region(struct q6v5_wcss *wcss)
return 0;
}
+static int q6v5_wcss_init_clock(struct q6v5_wcss *wcss)
+{
+ int ret;
+
+ wcss->xo = devm_clk_get(wcss->dev, "xo");
+ if (IS_ERR(wcss->xo)) {
+ ret = PTR_ERR(wcss->xo);
+ if (ret != -EPROBE_DEFER)
+ dev_err(wcss->dev, "failed to get xo clock");
+ return ret;
+ }
+
+ wcss->gcc_abhs_cbcr = devm_clk_get(wcss->dev, "gcc_abhs_cbcr");
+ if (IS_ERR(wcss->gcc_abhs_cbcr)) {
+ ret = PTR_ERR(wcss->gcc_abhs_cbcr);
+ if (ret != -EPROBE_DEFER)
+ dev_err(wcss->dev, "failed to get gcc abhs clock");
+ return PTR_ERR(wcss->gcc_abhs_cbcr);
+ }
+
+ wcss->gcc_axim_cbcr = devm_clk_get(wcss->dev, "gcc_axim_cbcr");
+ if (IS_ERR(wcss->gcc_axim_cbcr)) {
+ ret = PTR_ERR(wcss->gcc_axim_cbcr);
+ if (ret != -EPROBE_DEFER)
+ dev_err(wcss->dev, "failed to get gcc axim clock\n");
+ return PTR_ERR(wcss->gcc_axim_cbcr);
+ }
+
+ wcss->ahbfabric_cbcr_clk = devm_clk_get(wcss->dev,
+ "lcc_ahbfabric_cbc");
+ if (IS_ERR(wcss->ahbfabric_cbcr_clk)) {
+ ret = PTR_ERR(wcss->ahbfabric_cbcr_clk);
+ if (ret != -EPROBE_DEFER)
+ dev_err(wcss->dev, "failed to get ahbfabric clock\n");
+ return PTR_ERR(wcss->ahbfabric_cbcr_clk);
+ }
+
+ wcss->lcc_csr_cbcr = devm_clk_get(wcss->dev, "tcsr_lcc_cbc");
+ if (IS_ERR(wcss->lcc_csr_cbcr)) {
+ ret = PTR_ERR(wcss->lcc_csr_cbcr);
+ if (ret != -EPROBE_DEFER)
+ dev_err(wcss->dev, "failed to get csr cbcr clk\n");
+ return PTR_ERR(wcss->lcc_csr_cbcr);
+ }
+
+ wcss->ahbs_cbcr = devm_clk_get(wcss->dev,
+ "lcc_abhs_cbc");
+ if (IS_ERR(wcss->ahbs_cbcr)) {
+ ret = PTR_ERR(wcss->ahbs_cbcr);
+ if (ret != -EPROBE_DEFER)
+ dev_err(wcss->dev, "failed to get ahbs_cbcr clk\n");
+ return PTR_ERR(wcss->ahbs_cbcr);
+ }
+
+ wcss->tcm_slave_cbcr = devm_clk_get(wcss->dev,
+ "lcc_tcm_slave_cbc");
+ if (IS_ERR(wcss->tcm_slave_cbcr)) {
+ ret = PTR_ERR(wcss->tcm_slave_cbcr);
+ if (ret != -EPROBE_DEFER)
+ dev_err(wcss->dev, "failed to get tcm cbcr clk\n");
+ return PTR_ERR(wcss->tcm_slave_cbcr);
+ }
+
+ wcss->qdsp6ss_abhm_cbcr = devm_clk_get(wcss->dev, "lcc_abhm_cbc");
+ if (IS_ERR(wcss->qdsp6ss_abhm_cbcr)) {
+ ret = PTR_ERR(wcss->qdsp6ss_abhm_cbcr);
+ if (ret != -EPROBE_DEFER)
+ dev_err(wcss->dev, "failed to get abhm cbcr clk\n");
+ return PTR_ERR(wcss->qdsp6ss_abhm_cbcr);
+ }
+
+ wcss->qdsp6ss_axim_cbcr = devm_clk_get(wcss->dev, "lcc_axim_cbc");
+ if (IS_ERR(wcss->qdsp6ss_axim_cbcr)) {
+ ret = PTR_ERR(wcss->qdsp6ss_axim_cbcr);
+ if (ret != -EPROBE_DEFER)
+ dev_err(wcss->dev, "failed to get axim cbcr clk\n");
+ return PTR_ERR(wcss->qdsp6ss_abhm_cbcr);
+ }
+
+ wcss->lcc_bcr_sleep = devm_clk_get(wcss->dev, "lcc_bcr_sleep");
+ if (IS_ERR(wcss->lcc_bcr_sleep)) {
+ ret = PTR_ERR(wcss->lcc_bcr_sleep);
+ if (ret != -EPROBE_DEFER)
+ dev_err(wcss->dev, "failed to get bcr cbcr clk\n");
+ return PTR_ERR(wcss->lcc_bcr_sleep);
+ }
+
+ return 0;
+}
+
+static int q6v5_wcss_init_regulator(struct q6v5_wcss *wcss)
+{
+ wcss->cx_supply = devm_regulator_get(wcss->dev, "cx");
+ if (IS_ERR(wcss->cx_supply))
+ return PTR_ERR(wcss->cx_supply);
+
+ regulator_set_load(wcss->cx_supply, 100000);
+
+ return 0;
+}
+
static int q6v5_wcss_probe(struct platform_device *pdev)
{
const struct wcss_data *desc;
@@ -544,7 +1001,7 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
if (!desc)
return -EINVAL;
- rproc = rproc_alloc(&pdev->dev, pdev->name, &q6v5_wcss_ipq8074_ops,
+ rproc = rproc_alloc(&pdev->dev, pdev->name, desc->ops,
desc->firmware_name, sizeof(*wcss));
if (!rproc) {
dev_err(&pdev->dev, "failed to allocate rproc\n");
@@ -553,7 +1010,10 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
wcss = rproc->priv;
wcss->dev = &pdev->dev;
- wcss->crash_reason_smem = desc->crash_reason_smem;
+ wcss->version = desc->version;
+
+ wcss->version = desc->version;
+ wcss->requires_force_stop = desc->requires_force_stop;
ret = q6v5_wcss_init_mmio(wcss, pdev);
if (ret)
@@ -563,7 +1023,17 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
if (ret)
goto free_rproc;
- ret = q6v5_wcss_init_reset(wcss);
+ if (wcss->version == WCSS_QCS404) {
+ ret = q6v5_wcss_init_clock(wcss);
+ if (ret)
+ goto free_rproc;
+
+ ret = q6v5_wcss_init_regulator(wcss);
+ if (ret)
+ goto free_rproc;
+ }
+
+ ret = q6v5_wcss_init_reset(wcss, desc);
if (ret)
goto free_rproc;
@@ -572,6 +1042,12 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
if (ret)
goto free_rproc;
+ qcom_add_glink_subdev(rproc, &wcss->glink_subdev);
+ qcom_add_ssr_subdev(rproc, &wcss->ssr_subdev, desc->ssr_name);
+ wcss->sysmon = qcom_add_sysmon_subdev(rproc,
+ desc->sysmon_name,
+ desc->ssctl_id);
+
ret = rproc_add(rproc);
if (ret)
goto free_rproc;
@@ -599,10 +1075,28 @@ static int q6v5_wcss_remove(struct platform_device *pdev)
static const struct wcss_data wcss_ipq8074_res_init = {
.firmware_name = "IPQ8074/q6_fw.mdt",
.crash_reason_smem = WCSS_CRASH_REASON,
+ .aon_reset_required = true,
+ .wcss_q6_reset_required = true,
+ .ops = &q6v5_wcss_ipq8074_ops,
+ .requires_force_stop = true,
+};
+
+static const struct wcss_data wcss_qcs404_res_init = {
+ .crash_reason_smem = WCSS_CRASH_REASON,
+ .firmware_name = "wcnss.mdt",
+ .version = WCSS_QCS404,
+ .aon_reset_required = false,
+ .wcss_q6_reset_required = false,
+ .ssr_name = "mpss",
+ .sysmon_name = "wcnss",
+ .ssctl_id = 0x12,
+ .ops = &q6v5_wcss_qcs404_ops,
+ .requires_force_stop = false,
};
static const struct of_device_id q6v5_wcss_of_match[] = {
{ .compatible = "qcom,ipq8074-wcss-pil", .data = &wcss_ipq8074_res_init },
+ { .compatible = "qcom,qcs404-wcss-pil", .data = &wcss_qcs404_res_init },
{ },
};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH v5 5/7] remoteproc: qcom: wcss: populate hardcoded param using driver data
From: Govind Singh @ 2019-07-26 9:23 UTC (permalink / raw)
To: bjorn.andersson, linux-remoteproc, sboyd, linux-clk,
linux-arm-msm
Cc: andy.gross, linux-soc, devicetree, Govind Singh
In-Reply-To: <20190726092332.25202-1-govinds@codeaurora.org>
Q6 based WiFi fw loading is supported across
different targets, ex: IPQ8074/QCS404. In order to
support different fw names/pas id etc, populate
hardcoded param using driver data.
Signed-off-by: Govind Singh <govinds@codeaurora.org>
---
drivers/remoteproc/qcom_q6v5_wcss.c | 31 ++++++++++++++++++++++++-----
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index f93e1e4a1cc0..84accf1755e9 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -8,6 +8,7 @@
#include <linux/kernel.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
+#include <linux/of_device.h>
#include <linux/of_reserved_mem.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
@@ -70,6 +71,11 @@
#define TCSR_WCSS_CLK_MASK 0x1F
#define TCSR_WCSS_CLK_ENABLE 0x14
+struct wcss_data {
+ const char *firmware_name;
+ int crash_reason_smem;
+};
+
struct q6v5_wcss {
struct device *dev;
@@ -91,6 +97,8 @@ struct q6v5_wcss {
phys_addr_t mem_reloc;
void *mem_region;
size_t mem_size;
+
+ int crash_reason_smem;
};
static int q6v5_wcss_reset(struct q6v5_wcss *wcss)
@@ -427,7 +435,7 @@ static int q6v5_wcss_load(struct rproc *rproc, const struct firmware *fw)
wcss->mem_size, &wcss->mem_reloc);
}
-static const struct rproc_ops q6v5_wcss_ops = {
+static const struct rproc_ops q6v5_wcss_ipq8074_ops = {
.start = q6v5_wcss_start,
.stop = q6v5_wcss_stop,
.da_to_va = q6v5_wcss_da_to_va,
@@ -527,12 +535,17 @@ static int q6v5_alloc_memory_region(struct q6v5_wcss *wcss)
static int q6v5_wcss_probe(struct platform_device *pdev)
{
+ const struct wcss_data *desc;
struct q6v5_wcss *wcss;
struct rproc *rproc;
int ret;
- rproc = rproc_alloc(&pdev->dev, pdev->name, &q6v5_wcss_ops,
- "IPQ8074/q6_fw.mdt", sizeof(*wcss));
+ desc = of_device_get_match_data(&pdev->dev);
+ if (!desc)
+ return -EINVAL;
+
+ rproc = rproc_alloc(&pdev->dev, pdev->name, &q6v5_wcss_ipq8074_ops,
+ desc->firmware_name, sizeof(*wcss));
if (!rproc) {
dev_err(&pdev->dev, "failed to allocate rproc\n");
return -ENOMEM;
@@ -540,6 +553,7 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
wcss = rproc->priv;
wcss->dev = &pdev->dev;
+ wcss->crash_reason_smem = desc->crash_reason_smem;
ret = q6v5_wcss_init_mmio(wcss, pdev);
if (ret)
@@ -553,7 +567,8 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
if (ret)
goto free_rproc;
- ret = qcom_q6v5_init(&wcss->q6v5, pdev, rproc, WCSS_CRASH_REASON, NULL);
+ ret = qcom_q6v5_init(&wcss->q6v5, pdev, rproc, desc->crash_reason_smem,
+ NULL);
if (ret)
goto free_rproc;
@@ -581,8 +596,14 @@ static int q6v5_wcss_remove(struct platform_device *pdev)
return 0;
}
+static const struct wcss_data wcss_ipq8074_res_init = {
+ .firmware_name = "IPQ8074/q6_fw.mdt",
+ .crash_reason_smem = WCSS_CRASH_REASON,
+};
+
static const struct of_device_id q6v5_wcss_of_match[] = {
- { .compatible = "qcom,ipq8074-wcss-pil" },
+ { .compatible = "qcom,ipq8074-wcss-pil", .data = &wcss_ipq8074_res_init },
+
{ },
};
MODULE_DEVICE_TABLE(of, q6v5_wcss_of_match);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH v5 4/7] clk: qcom: Add Q6SSTOP clock controller for QCS404
From: Govind Singh @ 2019-07-26 9:23 UTC (permalink / raw)
To: bjorn.andersson, linux-remoteproc, sboyd, linux-clk,
linux-arm-msm
Cc: andy.gross, linux-soc, devicetree, Govind Singh
In-Reply-To: <20190726092332.25202-1-govinds@codeaurora.org>
Add support for the Q6SSTOP clock control used on qcs404
based devices. This would allow wcss remoteproc driver to
control the required WCSS Q6SSTOP clock/reset controls to
bring the subsystem out of reset and shutdown the WCSS Q6DSP.
Signed-off-by: Govind Singh <govinds@codeaurora.org>
---
drivers/clk/qcom/Kconfig | 8 +
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/q6sstop-qcs404.c | 223 ++++++++++++++++++
.../dt-bindings/clock/qcom,q6sstopcc-qcs404.h | 18 ++
4 files changed, 250 insertions(+)
create mode 100644 drivers/clk/qcom/q6sstop-qcs404.c
create mode 100644 include/dt-bindings/clock/qcom,q6sstopcc-qcs404.h
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index e1ff83cc361e..1b3c87a97521 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -250,6 +250,14 @@ config QCS_TURING_404
Support for the Turing Clock Controller on QCS404, provides clocks
and resets for the Turing subsystem.
+config QCS_Q6SSTOP_404
+ tristate "QCS404 Q6SSTOP Clock Controller"
+ select QCS_GCC_404
+ help
+ Support for the Q6SSTOP clock controller on QCS404 devices.
+ Say Y if you want to use the Q6SSTOP branch clocks of the WCSS clock
+ controller to reset the Q6SSTOP subsystem.
+
config SDM_GCC_845
tristate "SDM845 Global Clock Controller"
select QCOM_GDSC
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index f0768fb1f037..086c053e0e03 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_QCOM_CLK_RPMH) += clk-rpmh.o
obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
obj-$(CONFIG_QCS_GCC_404) += gcc-qcs404.o
obj-$(CONFIG_QCS_TURING_404) += turingcc-qcs404.o
+obj-$(CONFIG_QCS_Q6SSTOP_404) += q6sstop-qcs404.o
obj-$(CONFIG_SDM_CAMCC_845) += camcc-sdm845.o
obj-$(CONFIG_SDM_DISPCC_845) += dispcc-sdm845.o
obj-$(CONFIG_SDM_GCC_660) += gcc-sdm660.o
diff --git a/drivers/clk/qcom/q6sstop-qcs404.c b/drivers/clk/qcom/q6sstop-qcs404.c
new file mode 100644
index 000000000000..b6281986b077
--- /dev/null
+++ b/drivers/clk/qcom/q6sstop-qcs404.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/qcom,q6sstopcc-qcs404.h>
+
+#include "clk-regmap.h"
+#include "clk-branch.h"
+#include "common.h"
+#include "reset.h"
+
+static struct clk_branch lcc_ahbfabric_cbc_clk = {
+ .halt_reg = 0x1b004,
+ .halt_check = BRANCH_HALT,
+ .clkr = {
+ .enable_reg = 0x1b004,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "lcc_ahbfabric_cbc_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch lcc_q6ss_ahbs_cbc_clk = {
+ .halt_reg = 0x22000,
+ .halt_check = BRANCH_VOTED,
+ .clkr = {
+ .enable_reg = 0x22000,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "lcc_q6ss_ahbs_cbc_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch lcc_q6ss_tcm_slave_cbc_clk = {
+ .halt_reg = 0x1c000,
+ .halt_check = BRANCH_VOTED,
+ .clkr = {
+ .enable_reg = 0x1c000,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "lcc_q6ss_tcm_slave_cbc_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch lcc_q6ss_ahbm_cbc_clk = {
+ .halt_reg = 0x22004,
+ .halt_check = BRANCH_VOTED,
+ .clkr = {
+ .enable_reg = 0x22004,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "lcc_q6ss_ahbm_cbc_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch lcc_q6ss_axim_cbc_clk = {
+ .halt_reg = 0x1c004,
+ .halt_check = BRANCH_VOTED,
+ .clkr = {
+ .enable_reg = 0x1c004,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "lcc_q6ss_axim_cbc_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch lcc_q6ss_bcr_sleep_clk = {
+ .halt_reg = 0x6004,
+ .halt_check = BRANCH_VOTED,
+ .clkr = {
+ .enable_reg = 0x6004,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "lcc_q6ss_bcr_sleep_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+/* TCSR clock */
+static struct clk_branch tcsr_lcc_csr_cbcr_clk = {
+ .halt_reg = 0x8008,
+ .halt_check = BRANCH_VOTED,
+ .clkr = {
+ .enable_reg = 0x8008,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "tcsr_lcc_csr_cbcr_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct regmap_config q6sstop_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .fast_io = true,
+};
+
+static struct clk_regmap *q6sstop_qcs404_clocks[] = {
+ [LCC_AHBFABRIC_CBC_CLK] = &lcc_ahbfabric_cbc_clk.clkr,
+ [LCC_Q6SS_AHBS_CBC_CLK] = &lcc_q6ss_ahbs_cbc_clk.clkr,
+ [LCC_Q6SS_TCM_SLAVE_CBC_CLK] = &lcc_q6ss_tcm_slave_cbc_clk.clkr,
+ [LCC_Q6SS_AHBM_CBC_CLK] = &lcc_q6ss_ahbm_cbc_clk.clkr,
+ [LCC_Q6SS_AXIM_CBC_CLK] = &lcc_q6ss_axim_cbc_clk.clkr,
+ [LCC_Q6SS_BCR_SLEEP_CLK] = &lcc_q6ss_bcr_sleep_clk.clkr,
+};
+
+static const struct qcom_reset_map q6sstop_qcs404_resets[] = {
+ [Q6SSTOP_BCR_RESET] = { 0x6000 },
+};
+
+static const struct qcom_cc_desc q6sstop_qcs404_desc = {
+ .config = &q6sstop_regmap_config,
+ .clks = q6sstop_qcs404_clocks,
+ .num_clks = ARRAY_SIZE(q6sstop_qcs404_clocks),
+ .resets = q6sstop_qcs404_resets,
+ .num_resets = ARRAY_SIZE(q6sstop_qcs404_resets),
+};
+
+static struct clk_regmap *tcsr_qcs404_clocks[] = {
+ [TCSR_Q6SS_LCC_CBCR_CLK] = &tcsr_lcc_csr_cbcr_clk.clkr,
+};
+
+static const struct qcom_cc_desc tcsr_qcs404_desc = {
+ .config = &q6sstop_regmap_config,
+ .clks = tcsr_qcs404_clocks,
+ .num_clks = ARRAY_SIZE(tcsr_qcs404_clocks),
+};
+
+static const struct of_device_id q6sstopcc_qcs404_match_table[] = {
+ { .compatible = "qcom,qcs404-q6sstopcc" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, q6sstopcc_qcs404_match_table);
+
+static int q6sstopcc_qcs404_probe(struct platform_device *pdev)
+{
+ const struct qcom_cc_desc *desc;
+ int ret;
+
+ pm_runtime_enable(&pdev->dev);
+ ret = pm_clk_create(&pdev->dev);
+ if (ret)
+ goto disable_pm_runtime;
+
+ ret = pm_clk_add(&pdev->dev, NULL);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to acquire iface clock\n");
+ goto destroy_pm_clk;
+ }
+
+ q6sstop_regmap_config.name = "q6sstop_tcsr";
+ desc = &tcsr_qcs404_desc;
+
+ ret = qcom_cc_probe_by_index(pdev, 1, desc);
+ if (ret)
+ goto destroy_pm_clk;
+
+ q6sstop_regmap_config.name = "q6sstop_cc";
+ desc = &q6sstop_qcs404_desc;
+
+ ret = qcom_cc_probe_by_index(pdev, 0, desc);
+ if (ret)
+ goto destroy_pm_clk;
+
+ return 0;
+
+destroy_pm_clk:
+ pm_clk_destroy(&pdev->dev);
+
+disable_pm_runtime:
+ pm_runtime_disable(&pdev->dev);
+
+ return ret;
+}
+
+static int q6sstopcc_qcs404_remove(struct platform_device *pdev)
+{
+ pm_clk_destroy(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+
+ return 0;
+}
+
+static const struct dev_pm_ops q6sstopcc_pm_ops = {
+ SET_RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
+};
+
+static struct platform_driver q6sstopcc_qcs404_driver = {
+ .probe = q6sstopcc_qcs404_probe,
+ .remove = q6sstopcc_qcs404_remove,
+ .driver = {
+ .name = "qcs404-q6sstopcc",
+ .of_match_table = q6sstopcc_qcs404_match_table,
+ .pm = &q6sstopcc_pm_ops,
+ },
+};
+
+module_platform_driver(q6sstopcc_qcs404_driver);
+
+MODULE_DESCRIPTION("QTI QCS404 Q6SSTOP Clock Controller Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/dt-bindings/clock/qcom,q6sstopcc-qcs404.h b/include/dt-bindings/clock/qcom,q6sstopcc-qcs404.h
new file mode 100644
index 000000000000..c6f5290f0914
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,q6sstopcc-qcs404.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLK_Q6SSTOP_QCS404_H
+#define _DT_BINDINGS_CLK_Q6SSTOP_QCS404_H
+
+#define LCC_AHBFABRIC_CBC_CLK 0
+#define LCC_Q6SS_AHBS_CBC_CLK 1
+#define LCC_Q6SS_TCM_SLAVE_CBC_CLK 2
+#define LCC_Q6SS_AHBM_CBC_CLK 3
+#define LCC_Q6SS_AXIM_CBC_CLK 4
+#define LCC_Q6SS_BCR_SLEEP_CLK 5
+#define TCSR_Q6SS_LCC_CBCR_CLK 6
+
+#define Q6SSTOP_BCR_RESET 1
+#endif
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH v5 3/7] clk: qcom: define probe by index API as common API
From: Govind Singh @ 2019-07-26 9:23 UTC (permalink / raw)
To: bjorn.andersson, linux-remoteproc, sboyd, linux-clk,
linux-arm-msm
Cc: andy.gross, linux-soc, devicetree, Govind Singh
In-Reply-To: <20190726092332.25202-1-govinds@codeaurora.org>
Extend the probe by index API in common code to be used
by other qcom clock controller.
Signed-off-by: Govind Singh <govinds@codeaurora.org>
---
drivers/clk/qcom/common.c | 20 ++++++++++++++++++++
drivers/clk/qcom/common.h | 2 ++
drivers/clk/qcom/lpasscc-sdm845.c | 23 ++---------------------
3 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index a6b2f86112d8..28ddc747d703 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -306,4 +306,24 @@ int qcom_cc_probe(struct platform_device *pdev, const struct qcom_cc_desc *desc)
}
EXPORT_SYMBOL_GPL(qcom_cc_probe);
+int qcom_cc_probe_by_index(struct platform_device *pdev, int index,
+ const struct qcom_cc_desc *desc)
+{
+ struct regmap *regmap;
+ struct resource *res;
+ void __iomem *base;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, index);
+ base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(base))
+ return -ENOMEM;
+
+ regmap = devm_regmap_init_mmio(&pdev->dev, base, desc->config);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ return qcom_cc_really_probe(pdev, desc, regmap);
+}
+EXPORT_SYMBOL_GPL(qcom_cc_probe_by_index);
+
MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
index 1e2a8bdac55a..bb39a7e106d8 100644
--- a/drivers/clk/qcom/common.h
+++ b/drivers/clk/qcom/common.h
@@ -61,5 +61,7 @@ extern int qcom_cc_really_probe(struct platform_device *pdev,
struct regmap *regmap);
extern int qcom_cc_probe(struct platform_device *pdev,
const struct qcom_cc_desc *desc);
+extern int qcom_cc_probe_by_index(struct platform_device *pdev, int index,
+ const struct qcom_cc_desc *desc);
#endif
diff --git a/drivers/clk/qcom/lpasscc-sdm845.c b/drivers/clk/qcom/lpasscc-sdm845.c
index e246b99dfbc6..56d3e9928892 100644
--- a/drivers/clk/qcom/lpasscc-sdm845.c
+++ b/drivers/clk/qcom/lpasscc-sdm845.c
@@ -112,25 +112,6 @@ static const struct qcom_cc_desc lpass_qdsp6ss_sdm845_desc = {
.num_clks = ARRAY_SIZE(lpass_qdsp6ss_sdm845_clocks),
};
-static int lpass_clocks_sdm845_probe(struct platform_device *pdev, int index,
- const struct qcom_cc_desc *desc)
-{
- struct regmap *regmap;
- struct resource *res;
- void __iomem *base;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, index);
- base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
-
- regmap = devm_regmap_init_mmio(&pdev->dev, base, desc->config);
- if (IS_ERR(regmap))
- return PTR_ERR(regmap);
-
- return qcom_cc_really_probe(pdev, desc, regmap);
-}
-
static int lpass_cc_sdm845_probe(struct platform_device *pdev)
{
const struct qcom_cc_desc *desc;
@@ -139,14 +120,14 @@ static int lpass_cc_sdm845_probe(struct platform_device *pdev)
lpass_regmap_config.name = "cc";
desc = &lpass_cc_sdm845_desc;
- ret = lpass_clocks_sdm845_probe(pdev, 0, desc);
+ ret = qcom_cc_probe_by_index(pdev, 0, desc);
if (ret)
return ret;
lpass_regmap_config.name = "qdsp6ss";
desc = &lpass_qdsp6ss_sdm845_desc;
- return lpass_clocks_sdm845_probe(pdev, 1, desc);
+ return qcom_cc_probe_by_index(pdev, 1, desc);
}
static const struct of_device_id lpass_cc_sdm845_match_table[] = {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH v5 2/7] dt-bindings: clock: qcom: Add QCOM Q6SSTOP clock controller bindings
From: Govind Singh @ 2019-07-26 9:23 UTC (permalink / raw)
To: bjorn.andersson, linux-remoteproc, sboyd, linux-clk,
linux-arm-msm
Cc: andy.gross, linux-soc, devicetree, Govind Singh
In-Reply-To: <20190726092332.25202-1-govinds@codeaurora.org>
Add devicetree binding for the Q6SSTOP clock controller found in QCS404.
Signed-off-by: Govind Singh <govinds@codeaurora.org>
---
.../bindings/clock/qcom,q6sstopcc.txt | 26 +++++++++++++++++++
1 file changed, 26 insertions(+)
create mode 100755 Documentation/devicetree/bindings/clock/qcom,q6sstopcc.txt
diff --git a/Documentation/devicetree/bindings/clock/qcom,q6sstopcc.txt b/Documentation/devicetree/bindings/clock/qcom,q6sstopcc.txt
new file mode 100755
index 000000000000..157bb52f9dc4
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,q6sstopcc.txt
@@ -0,0 +1,26 @@
+Qualcomm Q6SSTOP Clock Controller Binding
+-----------------------------------------------
+
+Required properties :
+- compatible : shall contain "qcom,qcs404-q6sstopcc"
+- #clock-cells : from common clock binding, shall contain 1
+- reg : shall contain base register address and size,
+ in the order
+ Index 0 maps to Q6SSTOP clocks register region
+ Index 1 maps to Q6SSTOP_TCSR register region
+
+Optional properties :
+- reg-names : register names of WCSS domain
+ "q6sstop_cc", "q6sstop_tcsr".
+
+Example:
+The below node has to be defined in the cases where the WCSS peripheral loader
+would bring the subsystem out of reset.
+
+ q6sstopcc: clock-controller@7500000 {
+ compatible = "qcom,qcs404-q6sstopcc";
+ reg = <0x7500000 0x4e000>, <0x7550000 0x10000>;
+ reg-names = "q6sstop_cc", "q6sstop_tcsr";
+ clocks = <&gcc GCC_WCSS_Q6_AHB_CLK>;
+ #clock-cells = <1>;
+ };
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH v5 1/7] clk: qcom: Add WCSS gcc clock control for QCS404
From: Govind Singh @ 2019-07-26 9:23 UTC (permalink / raw)
To: bjorn.andersson, linux-remoteproc, sboyd, linux-clk,
linux-arm-msm
Cc: andy.gross, linux-soc, devicetree, Govind Singh
In-Reply-To: <20190726092332.25202-1-govinds@codeaurora.org>
Add support for the WCSS QDSP gcc clock control used on qcs404
based devices. This would allow wcss remoteproc driver to control
the required gcc clocks to bring the subsystem out of reset.
Signed-off-by: Govind Singh <govinds@codeaurora.org>
---
drivers/clk/qcom/gcc-qcs404.c | 30 +++++++++++++++++++++
include/dt-bindings/clock/qcom,gcc-qcs404.h | 3 +++
2 files changed, 33 insertions(+)
diff --git a/drivers/clk/qcom/gcc-qcs404.c b/drivers/clk/qcom/gcc-qcs404.c
index 29cf464dd2c8..e12c04c09a6a 100644
--- a/drivers/clk/qcom/gcc-qcs404.c
+++ b/drivers/clk/qcom/gcc-qcs404.c
@@ -2604,6 +2604,32 @@ static struct clk_branch gcc_usb_hs_system_clk = {
},
};
+static struct clk_branch gcc_wdsp_q6ss_ahbs_clk = {
+ .halt_reg = 0x1e004,
+ .halt_check = BRANCH_HALT,
+ .clkr = {
+ .enable_reg = 0x1e004,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gcc_wdsp_q6ss_ahbs_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch gcc_wdsp_q6ss_axim_clk = {
+ .halt_reg = 0x1e008,
+ .halt_check = BRANCH_HALT,
+ .clkr = {
+ .enable_reg = 0x1e008,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gcc_wdsp_q6ss_axim_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
static struct clk_hw *gcc_qcs404_hws[] = {
&cxo.hw,
};
@@ -2749,6 +2775,9 @@ static struct clk_regmap *gcc_qcs404_clocks[] = {
[GCC_QDSS_DAP_CLK] = &gcc_qdss_dap_clk.clkr,
[GCC_DCC_CLK] = &gcc_dcc_clk.clkr,
[GCC_DCC_XO_CLK] = &gcc_dcc_xo_clk.clkr,
+ [GCC_WCSS_Q6_AHB_CLK] = &gcc_wdsp_q6ss_ahbs_clk.clkr,
+ [GCC_WCSS_Q6_AXIM_CLK] = &gcc_wdsp_q6ss_axim_clk.clkr,
+
};
static const struct qcom_reset_map gcc_qcs404_resets[] = {
@@ -2774,6 +2803,7 @@ static const struct qcom_reset_map gcc_qcs404_resets[] = {
[GCC_PCIE_0_SLEEP_ARES] = { 0x3e040, 1 },
[GCC_PCIE_0_PIPE_ARES] = { 0x3e040, 0 },
[GCC_EMAC_BCR] = { 0x4e000 },
+ [GCC_WDSP_RESTART] = {0x19000},
};
static const struct regmap_config gcc_qcs404_regmap_config = {
diff --git a/include/dt-bindings/clock/qcom,gcc-qcs404.h b/include/dt-bindings/clock/qcom,gcc-qcs404.h
index 2cd62c98561f..bc3051543347 100644
--- a/include/dt-bindings/clock/qcom,gcc-qcs404.h
+++ b/include/dt-bindings/clock/qcom,gcc-qcs404.h
@@ -146,6 +146,8 @@
#define GCC_MDP_TBU_CLK 138
#define GCC_QDSS_DAP_CLK 139
#define GCC_DCC_XO_CLK 140
+#define GCC_WCSS_Q6_AHB_CLK 141
+#define GCC_WCSS_Q6_AXIM_CLK 142
#define GCC_CDSP_CFG_AHB_CLK 143
#define GCC_BIMC_CDSP_CLK 144
#define GCC_CDSP_TBU_CLK 145
@@ -173,5 +175,6 @@
#define GCC_PCIE_0_CORE_STICKY_ARES 19
#define GCC_PCIE_0_SLEEP_ARES 20
#define GCC_PCIE_0_PIPE_ARES 21
+#define GCC_WDSP_RESTART 22
#endif
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH v5 0/7] Add non PAS wcss Q6 support for QCS404
From: Govind Singh @ 2019-07-26 9:23 UTC (permalink / raw)
To: bjorn.andersson, linux-remoteproc, sboyd, linux-clk,
linux-arm-msm
Cc: andy.gross, linux-soc, devicetree, Govind Singh
Add non PAS(Peripheral Authentication System) WCSS remoteproc
driver support and wcss clock/reset controls for QCS404 SOC.
Add WCSS q6 bootup and shutdown sequence handled from
Application Processor SubSystem(APSS).
Changes since v4:
removed QDSP6SS clock controls and moved to wcss rproc.
renamed wcsccc to q6sstopcc.
cleanup/refactoring.
Changes since v3:
dt binding cleanup.
Fixed remoteproc recovery.
Added remoteproc dump support.
wcsscc cleanup/refactoring.
Changes since v2:
Removed unused properties.
Refactored code to have common logic b/w ipq8074 and QCS404, wherever possible.
Added compatible example.
Removed wcss-protected bool.
Changes since v1:
Corrected clock names as per comments in v1 patch.
Govind Singh (7):
clk: qcom: Add WCSS gcc clock control for QCS404
dt-bindings: clock: qcom: Add QCOM Q6SSTOP clock controller bindings
clk: qcom: define probe by index API as common API
clk: qcom: Add Q6SSTOP clock controller for QCS404
remoteproc: qcom: wcss: populate hardcoded param using driver data
remoteproc: qcom: wcss: Add non pas wcss Q6 support for QCS404
remoteproc: qcom: wcss: explicitly request exclusive reset control
.../bindings/clock/qcom,q6sstopcc.txt | 26 +
drivers/clk/qcom/Kconfig | 8 +
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/common.c | 20 +
drivers/clk/qcom/common.h | 2 +
drivers/clk/qcom/gcc-qcs404.c | 30 +
drivers/clk/qcom/lpasscc-sdm845.c | 23 +-
drivers/clk/qcom/q6sstop-qcs404.c | 223 +++++++
drivers/remoteproc/qcom_q6v5_wcss.c | 599 ++++++++++++++++--
include/dt-bindings/clock/qcom,gcc-qcs404.h | 3 +
.../dt-bindings/clock/qcom,q6sstopcc-qcs404.h | 18 +
11 files changed, 890 insertions(+), 63 deletions(-)
create mode 100755 Documentation/devicetree/bindings/clock/qcom,q6sstopcc.txt
create mode 100644 drivers/clk/qcom/q6sstop-qcs404.c
create mode 100644 include/dt-bindings/clock/qcom,q6sstopcc-qcs404.h
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: display/bridge: Add binding for IMX NWL mipi dsi host controller
From: Sam Ravnborg @ 2019-07-26 9:23 UTC (permalink / raw)
To: Guido Günther
Cc: Mark Rutland, devicetree, Jernej Skrabec, Pengutronix Kernel Team,
Neil Armstrong, David Airlie, Jonas Karlman, linux-kernel,
dri-devel, Rob Herring, NXP Linux Team, Robert Chiras, Lee Jones,
Shawn Guo, linux-arm-kernel, Laurent Pinchart
In-Reply-To: <70a5c6617936a4a095e7608b96e3f9fae5ddfbb1.1563983037.git.agx@sigxcpu.org>
Hi Guido.
A few comments follows.
Sam
On Wed, Jul 24, 2019 at 05:52:25PM +0200, Guido Günther wrote:
> The Northwest Logic MIPI DSI IP core can be found in NXPs i.MX8 SoCs.
>
> Signed-off-by: Guido Günther <agx@sigxcpu.org>
> ---
> .../bindings/display/bridge/imx-nwl-dsi.txt | 89 +++++++++++++++++++
New binding. Any chance we can get this in yaml format?
This is the way forward and we have to convert the file anyway.
None of the other bridges use yaml format, but someone has to be the
first.
> 1 file changed, 89 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/bridge/imx-nwl-dsi.txt
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/imx-nwl-dsi.txt b/Documentation/devicetree/bindings/display/bridge/imx-nwl-dsi.txt
> new file mode 100644
> index 000000000000..288fdb726d5a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/imx-nwl-dsi.txt
> @@ -0,0 +1,89 @@
> +Northwest Logic MIPI-DSI on imx SoCs
> +=====================================
> +
> +NWL MIPI-DSI host controller found on i.MX8 platforms. This is a
> +dsi bridge for the for the NWL MIPI-DSI host.
To my best understanding a bridge is something that converts from one
format to another format.
Something that in the drm world are connected to an encoder.
I do not know the HW here - but from this very brif description this
sounds more like a display controller and not a bridge?
> +
> +Required properties:
> +- compatible: "fsl,<chip>-nwl-dsi"
> + The following strings are expected:
> + "fsl,imx8mq-nwl-dsi"
> +- reg: the register range of the MIPI-DSI controller
> +- interrupts: the interrupt number for this module
> +- clock, clock-names: phandles to the MIPI-DSI clocks
> + The following clocks are expected on all platforms:
> + "core" - DSI core clock
> + "tx_esc" - TX_ESC clock (used in escape mode)
> + "rx_esc" - RX_ESC clock (used in escape mode)
> + "phy_ref" - PHY_REF clock. Clock is managed by the phy. Only
> + used to read the clock rate.
> +- assigned-clocks: phandles to clocks that require initial configuration
> +- assigned-clock-rates: rates of the clocks that require initial configuration
> + The following clocks need to have an initial configuration:
> + "tx_esc" (20 MHz) and "rx_esc" (80 Mhz).
> +- phys: phandle to the phy module representing the DPHY
> + inside the MIPI-DSI IP block
> +- phy-names: should be "dphy"
> +
> +Optional properties:
> +- power-domains phandle to the power domain
> +- src phandle to the system reset controller (required on
> + i.MX8MQ)
Name is not very descriptive.
Other bindings seems to use "resets" here?
> +- mux-sel phandle to the MUX register set (required on i.MX8MQ)
> +- assigned-clock-parents phandles to parent clocks that needs to be assigned as
> + parents to clocks defined in assigned-clocks
> +
> +Example:
> + mipi_dsi: mipi_dsi@30a00000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "fsl,imx8mq-nwl-dsi";
> + reg = <0x30A00000 0x300>;
> + clocks = <&clk IMX8MQ_CLK_DSI_CORE>,
> + <&clk IMX8MQ_CLK_DSI_AHB>,
> + <&clk IMX8MQ_CLK_DSI_IPG_DIV>,
> + <&clk IMX8MQ_CLK_DSI_PHY_REF>;
> + clock-names = "core", "rx_esc", "tx_esc", "phy_ref";
> + assigned-clocks = <&clk IMX8MQ_CLK_DSI_AHB>,
> + <&clk IMX8MQ_CLK_DSI_CORE>,
> + <&clk IMX8MQ_CLK_DSI_IPG_DIV>;
> + assigned-clock-parents = <&clk IMX8MQ_SYS1_PLL_80M>,
> + <&clk IMX8MQ_SYS1_PLL_266M>;
> + assigned-clock-rates = <80000000>,
> + <266000000>,
> + <20000000>;
> + interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
> + power-domains = <&pgc_mipi>;
> + src = <&src>;
> + mux-sel = <&iomuxc_gpr>;
> + phys = <&dphy>;
> + phy-names = "dphy";
> + status = "okay";
I recall status should not be included in examples.
> +
> + panel@0 {
> + compatible = "...";
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <&mipi_dsi_out>;
> + };
> + };
> + };
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + mipi_dsi_in: endpoint {
> + remote-endpoint = <&dcss_disp0_mipi_dsi>;
> + };
> + };
> + port@1 {
> + reg = <1>;
> + mipi_dsi_out: endpoint {
> + remote-endpoint = <&panel_in>;
> + };
> + };
> + };
> + };
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH] ARM: dts: add device tree for Mecer Xtreme Mini S6
From: Justin Swartz @ 2019-07-26 9:14 UTC (permalink / raw)
To: Heiko Stuebner
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1618985.EOrKlNyPW4@phil>
Hi Heiko,
On 2019-07-26 00:19, Heiko Stuebner wrote:
> please add an entry to
> Documentation/devicetree/bindings/arm/rockchip.yaml
> for your board and if necessary also a vendor-prefix to
> Documentation/devicetree/bindings/vendor-prefixes.(yaml?)
OK
> please sort the &node-references alphabetically.
OK
>> +&cpu0 {
>> + clock-frequency = <1464000000>;
>
> not sure I understand the reasoning here.
> There seems to be a regulator defined, so the cpu cores should
> have operating points defined to allow them to switch between
> different frequencies as needed.
I added the clock-frequency property to quell the following messages:
[ 0.003273] /cpus/cpu@f00 missing clock-frequency property
[ 0.003323] /cpus/cpu@f01 missing clock-frequency property
[ 0.003352] /cpus/cpu@f02 missing clock-frequency property
[ 0.003382] /cpus/cpu@f03 missing clock-frequency property
I think they are from parse_dt_topology() in arch/arm/kernel/topology.c
What do you suggest?
Regards
Justin
^ permalink raw reply
* media: mtk-vcodec: Handle H264 error bitstreams
From: gtk_ruiwang @ 2019-07-26 8:54 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa, Tiffany Lin
Cc: Longfei Wang, Yunfei Dong, Mauro Carvalho Chehab,
Matthias Brugger, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, gtk_ruiwang
From: gtk_ruiwang <gtk_ruiwang@mediatek.com>
Error h264 bitstreams which picture info are out range of
decoder hardware specification, and no nal start code at the
beginning of the buffer, stop decoding and exit.
Signed-off-by: gtk_ruiwang <gtk_ruiwang@mediatek.com>
---
.../platform/mtk-vcodec/vdec/vdec_h264_if.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_if.c b/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_if.c
index c5f8f1fca44c..49aa85a9bb5a 100644
--- a/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_if.c
+++ b/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_if.c
@@ -29,6 +29,9 @@
#define H264_MAX_FB_NUM 17
#define HDR_PARSING_BUF_SZ 1024
+#define DEC_ERR_RET(ret) ((ret) >> 16)
+#define H264_ERR_NOT_VALID 3
+
/**
* struct h264_fb - h264 decode frame buffer information
* @vdec_fb_va : virtual address of struct vdec_fb
@@ -357,8 +360,11 @@ static int vdec_h264_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
buf = (unsigned char *)bs->va;
buf_sz = bs->size;
nal_start_idx = find_start_code(buf, buf_sz);
- if (nal_start_idx < 0)
+ if (nal_start_idx < 0) {
+ mtk_vcodec_err(inst, "invalid nal start code");
+ err = -EIO;
goto err_free_fb_out;
+ }
nal_start = buf[nal_start_idx];
nal_type = NAL_TYPE(buf[nal_start_idx]);
@@ -382,8 +388,14 @@ static int vdec_h264_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
data[0] = buf_sz;
data[1] = nal_start;
err = vpu_dec_start(vpu, data, 2);
- if (err)
+ if (err) {
+ if (err > 0 && (DEC_ERR_RET(err) == H264_ERR_NOT_VALID)) {
+ mtk_vcodec_err(inst, "- error bitstream - err = %d -",
+ err);
+ err = -EIO;
+ }
goto err_free_fb_out;
+ }
*res_chg = inst->vsi->dec.resolution_changed;
if (*res_chg) {
--
2.18.0
^ permalink raw reply related
* Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger
From: Petr Mladek @ 2019-07-26 8:31 UTC (permalink / raw)
To: Brendan Higgins
Cc: Jeff Dike, Kevin Hilman, Logan Gunthorpe, Michael Ellerman,
Daniel Vetter, Amir Goldstein, Frank Rowand, Steven Rostedt,
Kees Cook, David Rientjes, kunit-dev, Kieran Bingham,
Peter Zijlstra, Randy Dunlap, Joel Stanley, Luis Chamberlain,
Rob Herring, Stephen Boyd, shuah, wfg, Greg KH
In-Reply-To: <CAFd5g47v3Mr4GEGOjqyYy9Jwwm+ow7ypbu9j88rxEN06QCzdxQ@mail.gmail.com>
On Thu 2019-07-25 13:21:12, Brendan Higgins wrote:
> On Wed, Jul 24, 2019 at 12:31 AM Petr Mladek <pmladek@suse.com> wrote:
> >
> > On Mon 2019-07-22 16:54:10, Stephen Boyd wrote:
> > > Quoting Brendan Higgins (2019-07-22 15:30:49)
> > > > On Mon, Jul 22, 2019 at 1:03 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > > > >
> > > > >
> > > > > What's the calling context of the assertions and expectations? I still
> > > > > don't like the fact that string stream needs to allocate buffers and
> > > > > throw them into a list somewhere because the calling context matters
> > > > > there.
> > > >
> > > > The calling context is the same as before, which is anywhere.
> > >
> > > Ok. That's concerning then.
> > >
> > > >
> > > > > I'd prefer we just wrote directly to the console/log via printk
> > > > > instead. That way things are simple because we use the existing
> > > > > buffering path of printk, but maybe there's some benefit to the string
> > > > > stream that I don't see? Right now it looks like it builds a string and
> > > > > then dumps it to printk so I'm sort of lost what the benefit is over
> > > > > just writing directly with printk.
> > > >
> > > > It's just buffering it so the whole string gets printed uninterrupted.
> > > > If we were to print out piecemeal to printk, couldn't we have another
> > > > call to printk come in causing it to garble the KUnit message we are
> > > > in the middle of printing?
> > >
> > > Yes, printing piecemeal by calling printk many times could lead to
> > > interleaving of messages if something else comes in such as an interrupt
> > > printing something. Printk has some support to hold "records" but I'm
> > > not sure how that would work here because KERN_CONT talks about only
> > > being used early on in boot code. I haven't looked at printk in detail
> > > though so maybe I'm all wrong and KERN_CONT just works?
> >
> > KERN_CONT does not guarantee that the message will get printed
> > together. The pieces get interleaved with messages printed in
> > parallel.
> >
> > Note that KERN_CONT was originally really meant to be used only during
> > boot. It was later used more widely and ended in the best effort category.
> >
> > There were several attempts to make it more reliable. But it was
> > always either too complicated or error prone or both.
> >
> > You need to use your own buffering if you rely want perfect output.
> > The question is if it is really worth the complexity. Also note that
> > any buffering reduces the chance that the messages will reach
> > the console.
>
> Seems like that settles it then. Thanks!
>
> > BTW: There is a work in progress on a lockless printk ring buffer.
> > It will make printk() more secure regarding deadlocks. But it might
> > make transparent handling of continuous lines even more tricky.
> >
> > I guess that local buffering, before calling printk(), will be
> > even more important then. Well, it might really force us to create
> > an API for it.
>
> Cool! Can you CC me on that discussion?
Adding John Oggness into CC.
John, please CC Brendan Higgins on the patchsets eventually switching
printk() into the lockless buffer. The test framework is going to
do its own buffering to keep the related messages together.
The lockless ringbuffer might make handling of related (partial)
lines worse or better. It might justify KUnit's extra buffering
or it might allow to get rid of it.
> > Note that stroring the messages into the printk log is basically safe in any
> > context. It uses temporary per-CPU buffers for recursive messages and
> > in NMI. The only problem is panic() when some CPU gets stuck with the
> > lock taken. This will get solved by the lockless ringbuffer. Also
> > the temporary buffers will not be necessary any longer.
>
> Sure, I think Stephen's concern is all the supporting code that is
> involved. Not printk specifically. It just means a lot more of KUnit
> has to be IRQ safe.
I see.
BTW: I wonder if KUnit could reuse the existing seq_buf implementation
for buffering messages.
I am sorry if it has already been proposed and rejected for some
reason. I might have missed it. Feel free to just point me to
same older mail.
> > Much bigger problems are with consoles. There are many of them. It
> > means a lot of code and more locks involved, including scheduler
> > locks. Note that console lock is a semaphore.
>
> That shouldn't affect us though, right? As long as we continue to use
> the printk interface?
I guess that it should not affect KUnit.
The only problem might be if the testing framework calls printk()
inside scheduler or console code. And only when the tested code
uses the same locks that will be used by the called printk().
To be honest I do not fully understand KUnit design. I am not
completely sure how the tested code is isolated from the running
system. Namely, I do not know if the tested code shares
the same locks with the system running the test.
Best Regards,
Petr
^ permalink raw reply
* [PATCH v2 3/3] ARM: dts: exynos: Use standard arrays of generic PHYs for EHCI/OHCI devices
From: Marek Szyprowski @ 2019-07-26 8:14 UTC (permalink / raw)
To: linux-usb, linux-samsung-soc
Cc: linux-kernel, devicetree, Greg Kroah-Hartman, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Markus Reichl, Måns Rullgård,
Krzysztof Kozlowski, Peter Chen, Alan Stern, Rob Herring
In-Reply-To: <20190726081453.9456-1-m.szyprowski@samsung.com>
Move USB PHYs to a standard arrays for Exynos EHCI/OHCI devices. This
resolves the conflict between Exynos EHCI/OHCI sub-nodes and generic USB
device bindings. Once the Exynos EHCI/OHCI sub-nodes are removed, the
boards can finally provide sub-nodes for the USB devices using generic USB
device bindings.
Suggested-by: Måns Rullgård <mans@mansr.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
arch/arm/boot/dts/exynos4.dtsi | 28 +++----------------
.../boot/dts/exynos4210-universal_c210.dts | 8 ++----
arch/arm/boot/dts/exynos4412-itop-elite.dts | 9 ++----
arch/arm/boot/dts/exynos4412-odroidu3.dts | 8 ++----
arch/arm/boot/dts/exynos4412-odroidx.dts | 5 ++--
arch/arm/boot/dts/exynos4412-origen.dts | 9 ++----
arch/arm/boot/dts/exynos5250.dtsi | 16 +++--------
arch/arm/boot/dts/exynos54xx.dtsi | 18 +++---------
8 files changed, 22 insertions(+), 79 deletions(-)
diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index 1264cc431ff6..433f109d97ca 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -380,23 +380,8 @@
clocks = <&clock CLK_USB_HOST>;
clock-names = "usbhost";
status = "disabled";
- #address-cells = <1>;
- #size-cells = <0>;
- port@0 {
- reg = <0>;
- phys = <&exynos_usbphy 1>;
- status = "disabled";
- };
- port@1 {
- reg = <1>;
- phys = <&exynos_usbphy 2>;
- status = "disabled";
- };
- port@2 {
- reg = <2>;
- phys = <&exynos_usbphy 3>;
- status = "disabled";
- };
+ phys = <&exynos_usbphy 1>, <&exynos_usbphy 2>, <&exynos_usbphy 3>;
+ phy-names = "host", "hsic0", "hsic1";
};
ohci: ohci@12590000 {
@@ -406,13 +391,8 @@
clocks = <&clock CLK_USB_HOST>;
clock-names = "usbhost";
status = "disabled";
- #address-cells = <1>;
- #size-cells = <0>;
- port@0 {
- reg = <0>;
- phys = <&exynos_usbphy 1>;
- status = "disabled";
- };
+ phys = <&exynos_usbphy 1>;
+ phy-names = "host";
};
gpu: gpu@13000000 {
diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts b/arch/arm/boot/dts/exynos4210-universal_c210.dts
index 82a8b5449978..09d3d54d09ff 100644
--- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
+++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
@@ -204,9 +204,8 @@
&ehci {
status = "okay";
- port@0 {
- status = "okay";
- };
+ phys = <&exynos_usbphy 1>;
+ phy-names = "host";
};
&exynos_usbphy {
@@ -520,9 +519,6 @@
&ohci {
status = "okay";
- port@0 {
- status = "okay";
- };
};
&pinctrl_1 {
diff --git a/arch/arm/boot/dts/exynos4412-itop-elite.dts b/arch/arm/boot/dts/exynos4412-itop-elite.dts
index 0dedeba89b5f..f6d0a5f5d339 100644
--- a/arch/arm/boot/dts/exynos4412-itop-elite.dts
+++ b/arch/arm/boot/dts/exynos4412-itop-elite.dts
@@ -146,13 +146,8 @@
/* In order to reset USB ethernet */
samsung,vbus-gpio = <&gpc0 1 GPIO_ACTIVE_HIGH>;
- port@0 {
- status = "okay";
- };
-
- port@2 {
- status = "okay";
- };
+ phys = <&exynos_usbphy 1>, <&exynos_usbphy 3>;
+ phy-names = "host", "hsic1";
};
&exynos_usbphy {
diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts
index 96d99887bceb..8ff243ba4542 100644
--- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
@@ -105,12 +105,8 @@
};
&ehci {
- port@1 {
- status = "okay";
- };
- port@2 {
- status = "okay";
- };
+ phys = <&exynos_usbphy 2>, <&exynos_usbphy 3>;
+ phy-names = "hsic0", "hsic1";
};
&sound {
diff --git a/arch/arm/boot/dts/exynos4412-odroidx.dts b/arch/arm/boot/dts/exynos4412-odroidx.dts
index a2251581f6b6..3ea2a0101e80 100644
--- a/arch/arm/boot/dts/exynos4412-odroidx.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidx.dts
@@ -72,9 +72,8 @@
};
&ehci {
- port@1 {
- status = "okay";
- };
+ phys = <&exynos_usbphy 2>;
+ phy-names = "hsic0";
};
&mshc_0 {
diff --git a/arch/arm/boot/dts/exynos4412-origen.dts b/arch/arm/boot/dts/exynos4412-origen.dts
index 698de4345d16..ecd14b283a6b 100644
--- a/arch/arm/boot/dts/exynos4412-origen.dts
+++ b/arch/arm/boot/dts/exynos4412-origen.dts
@@ -88,13 +88,8 @@
&ehci {
samsung,vbus-gpio = <&gpx3 5 1>;
status = "okay";
-
- port@1 {
- status = "okay";
- };
- port@2 {
- status = "okay";
- };
+ phys = <&exynos_usbphy 2>, <&exynos_usbphy 3>;
+ phy-names = "hsic0", "hsic1";
};
&fimd {
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index d5e0392b409e..c5584f40ebfb 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -617,12 +617,8 @@
clocks = <&clock CLK_USB2>;
clock-names = "usbhost";
- #address-cells = <1>;
- #size-cells = <0>;
- port@0 {
- reg = <0>;
- phys = <&usb2_phy_gen 1>;
- };
+ phys = <&usb2_phy_gen 1>;
+ phy-names = "host";
};
ohci: usb@12120000 {
@@ -632,12 +628,8 @@
clocks = <&clock CLK_USB2>;
clock-names = "usbhost";
- #address-cells = <1>;
- #size-cells = <0>;
- port@0 {
- reg = <0>;
- phys = <&usb2_phy_gen 1>;
- };
+ phys = <&usb2_phy_gen 1>;
+ phy-names = "host";
};
usb2_phy_gen: phy@12130000 {
diff --git a/arch/arm/boot/dts/exynos54xx.dtsi b/arch/arm/boot/dts/exynos54xx.dtsi
index 0b27bebf9528..9c3b63b7cac6 100644
--- a/arch/arm/boot/dts/exynos54xx.dtsi
+++ b/arch/arm/boot/dts/exynos54xx.dtsi
@@ -189,26 +189,16 @@
compatible = "samsung,exynos4210-ehci";
reg = <0x12110000 0x100>;
interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
-
- #address-cells = <1>;
- #size-cells = <0>;
- port@0 {
- reg = <0>;
- phys = <&usb2_phy 1>;
- };
+ phys = <&usb2_phy 1>;
+ phy-names = "host";
};
usbhost1: usb@12120000 {
compatible = "samsung,exynos4210-ohci";
reg = <0x12120000 0x100>;
interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
-
- #address-cells = <1>;
- #size-cells = <0>;
- port@0 {
- reg = <0>;
- phys = <&usb2_phy 1>;
- };
+ phys = <&usb2_phy 1>;
+ phy-names = "host";
};
usb2_phy: phy@12130000 {
--
2.17.1
^ permalink raw reply related
* [PATCH v2 2/3] usb: exynos: add support for getting PHYs from the standard dt array
From: Marek Szyprowski @ 2019-07-26 8:14 UTC (permalink / raw)
To: linux-usb, linux-samsung-soc
Cc: linux-kernel, devicetree, Greg Kroah-Hartman, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Markus Reichl, Måns Rullgård,
Krzysztof Kozlowski, Peter Chen, Alan Stern, Rob Herring
In-Reply-To: <20190726081453.9456-1-m.szyprowski@samsung.com>
Add the code for getting generic PHYs from standard device tree array
from the main controller device node. This is a first step in resolving
the conflict between Exynos EHCI/OHCI sub-nodes and generic USB device
bindings. Later the sub-nodes currently used for assigning PHYs to root
ports of the controller will be removed making a place for the generic
USB device bindings nodes.
Suggested-by: Måns Rullgård <mans@mansr.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/usb/host/ehci-exynos.c | 23 +++++++++++++++++++----
drivers/usb/host/ohci-exynos.c | 23 +++++++++++++++++++----
2 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 3a29a1a8519c..01debfd03d4a 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -41,6 +41,7 @@ struct exynos_ehci_hcd {
struct clk *clk;
struct device_node *of_node;
struct phy *phy[PHY_NUMBER];
+ bool legacy_phy;
};
#define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
@@ -50,10 +51,22 @@ static int exynos_ehci_get_phy(struct device *dev,
{
struct device_node *child;
struct phy *phy;
- int phy_number;
+ int phy_number, num_phys;
int ret;
/* Get PHYs for the controller */
+ num_phys = of_count_phandle_with_args(dev->of_node, "phys",
+ "#phy-cells");
+ for (phy_number = 0; phy_number < num_phys; phy_number++) {
+ phy = devm_of_phy_get_by_index(dev, dev->of_node, phy_number);
+ if (IS_ERR(phy))
+ return PTR_ERR(phy);
+ exynos_ehci->phy[phy_number] = phy;
+ }
+ if (num_phys > 0)
+ return 0;
+
+ /* Get PHYs using legacy bindings */
for_each_available_child_of_node(dev->of_node, child) {
ret = of_property_read_u32(child, "reg", &phy_number);
if (ret) {
@@ -84,6 +97,7 @@ static int exynos_ehci_get_phy(struct device *dev,
}
}
+ exynos_ehci->legacy_phy = true;
return 0;
}
@@ -205,11 +219,12 @@ static int exynos_ehci_probe(struct platform_device *pdev)
ehci->caps = hcd->regs;
/*
- * Workaround: reset of_node pointer to avoid conflict between Exynos
- * EHCI port subnodes and generic USB device bindings
+ * Workaround: reset of_node pointer to avoid conflict between legacy
+ * Exynos EHCI port subnodes and generic USB device bindings
*/
exynos_ehci->of_node = pdev->dev.of_node;
- pdev->dev.of_node = NULL;
+ if (exynos_ehci->legacy_phy)
+ pdev->dev.of_node = NULL;
/* DMA burst Enable */
writel(EHCI_INSNREG00_ENABLE_DMA_BURST, EHCI_INSNREG00(hcd->regs));
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 905c6317e0c3..d5ce98e205c7 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -32,6 +32,7 @@ struct exynos_ohci_hcd {
struct clk *clk;
struct device_node *of_node;
struct phy *phy[PHY_NUMBER];
+ bool legacy_phy;
};
static int exynos_ohci_get_phy(struct device *dev,
@@ -39,10 +40,22 @@ static int exynos_ohci_get_phy(struct device *dev,
{
struct device_node *child;
struct phy *phy;
- int phy_number;
+ int phy_number, num_phys;
int ret;
/* Get PHYs for the controller */
+ num_phys = of_count_phandle_with_args(dev->of_node, "phys",
+ "#phy-cells");
+ for (phy_number = 0; phy_number < num_phys; phy_number++) {
+ phy = devm_of_phy_get_by_index(dev, dev->of_node, phy_number);
+ if (IS_ERR(phy))
+ return PTR_ERR(phy);
+ exynos_ohci->phy[phy_number] = phy;
+ }
+ if (num_phys > 0)
+ return 0;
+
+ /* Get PHYs using legacy bindings */
for_each_available_child_of_node(dev->of_node, child) {
ret = of_property_read_u32(child, "reg", &phy_number);
if (ret) {
@@ -73,6 +86,7 @@ static int exynos_ohci_get_phy(struct device *dev,
}
}
+ exynos_ohci->legacy_phy = true;
return 0;
}
@@ -172,11 +186,12 @@ static int exynos_ohci_probe(struct platform_device *pdev)
}
/*
- * Workaround: reset of_node pointer to avoid conflict between Exynos
- * OHCI port subnodes and generic USB device bindings
+ * Workaround: reset of_node pointer to avoid conflict between legacy
+ * Exynos OHCI port subnodes and generic USB device bindings
*/
exynos_ohci->of_node = pdev->dev.of_node;
- pdev->dev.of_node = NULL;
+ if (exynos_ohci->legacy_phy)
+ pdev->dev.of_node = NULL;
err = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (err) {
--
2.17.1
^ permalink raw reply related
* [PATCH v2 1/3] dt-bindings: switch Exynos EHCI/OHCI bindings to use array of generic PHYs
From: Marek Szyprowski @ 2019-07-26 8:14 UTC (permalink / raw)
To: linux-usb, linux-samsung-soc
Cc: linux-kernel, devicetree, Greg Kroah-Hartman, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Markus Reichl, Måns Rullgård,
Krzysztof Kozlowski, Peter Chen, Alan Stern, Rob Herring
In-Reply-To: <20190726081453.9456-1-m.szyprowski@samsung.com>
Commit 69bec7259853 ("USB: core: let USB device know device node") added
support for attaching devicetree node for USB devices. Those nodes are
children of their USB host controller. However Exynos EHCI and OHCI
driver bindings already define child-nodes for each physical root hub
port and assigns respective PHY controller and parameters to them. This
leads to the conflict. A workaround for it has been merged as commit
01d4071486fe ("usb: exynos: add workaround for the USB device bindings
conflict"), but it disabled support for USB device binding for Exynos
EHCI/OHCI controllers.
To resolve it properly, lets move PHYs from the sub-nodes to a standard
array under the 'phys' property.
Suggested-by: Måns Rullgård <mans@mansr.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
.../devicetree/bindings/usb/exynos-usb.txt | 41 +++++++------------
1 file changed, 14 insertions(+), 27 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index b7111f43fa59..66c394f9e11f 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -12,13 +12,11 @@ Required properties:
- interrupts: interrupt number to the cpu.
- clocks: from common clock binding: handle to usb clock.
- clock-names: from common clock binding: Shall be "usbhost".
- - port: if in the SoC there are EHCI phys, they should be listed here.
- One phy per port. Each port should have following entries:
- - reg: port number on EHCI controller, e.g
- On Exynos5250, port 0 is USB2.0 otg phy
- port 1 is HSIC phy0
- port 2 is HSIC phy1
- - phys: from the *Generic PHY* bindings; specifying phy used by port.
+ - phys: from the *Generic PHY* bindings; array specifying phy(s) used
+ by the root port.
+ - phy-names: from the *Generic PHY* bindings; array of the names for
+ each phy for the root ports, must be a subset of the following:
+ "host", "hsic0", "hsic1".
Optional properties:
- samsung,vbus-gpio: if present, specifies the GPIO that
@@ -35,12 +33,8 @@ Example:
clocks = <&clock 285>;
clock-names = "usbhost";
- #address-cells = <1>;
- #size-cells = <0>;
- port@0 {
- reg = <0>;
- phys = <&usb2phy 1>;
- };
+ phys = <&usb2phy 1>;
+ phy-names = "host";
};
OHCI
@@ -52,13 +46,11 @@ Required properties:
- interrupts: interrupt number to the cpu.
- clocks: from common clock binding: handle to usb clock.
- clock-names: from common clock binding: Shall be "usbhost".
- - port: if in the SoC there are OHCI phys, they should be listed here.
- One phy per port. Each port should have following entries:
- - reg: port number on OHCI controller, e.g
- On Exynos5250, port 0 is USB2.0 otg phy
- port 1 is HSIC phy0
- port 2 is HSIC phy1
- - phys: from the *Generic PHY* bindings, specifying phy used by port.
+ - phys: from the *Generic PHY* bindings; array specifying phy(s) used
+ by the root port.
+ - phy-names: from the *Generic PHY* bindings; array of the names for
+ each phy for the root ports, must be a subset of the following:
+ "host", "hsic0", "hsic1".
Example:
usb@12120000 {
@@ -69,13 +61,8 @@ Example:
clocks = <&clock 285>;
clock-names = "usbhost";
- #address-cells = <1>;
- #size-cells = <0>;
- port@0 {
- reg = <0>;
- phys = <&usb2phy 1>;
- };
-
+ phys = <&usb2phy 1>;
+ phy-names = "host";
};
DWC3
--
2.17.1
^ permalink raw reply related
* [PATCH v2 0/3] Exynos EHCI/OHCI: resolve conflict with the generic USB device bindings
From: Marek Szyprowski @ 2019-07-26 8:14 UTC (permalink / raw)
To: linux-usb, linux-samsung-soc
Cc: linux-kernel, devicetree, Greg Kroah-Hartman, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Markus Reichl, Måns Rullgård,
Krzysztof Kozlowski, Peter Chen, Alan Stern, Rob Herring
In-Reply-To: <CGME20190726081504eucas1p1aea5376ff300f6baca21c5e1cb6e4b43@eucas1p1.samsung.com>
Dear All,
Commit 69bec7259853 ("USB: core: let USB device know device node") added
support for attaching devicetree node for USB devices. Those nodes are
children of their USB host controller. However Exynos EHCI and OHCI
driver bindings already define child-nodes for each physical root hub
port and assigns respective PHY controller and parameters to them. This
leads to the conflict. A workaround for it has been merged as commit
01d4071486fe ("usb: exynos: add workaround for the USB device bindings
conflict"), but it disabled support for USB device binding for Exynos
EHCI/OHCI controllers.
This patchset tries to resolve this binding conflict by changing Exynos
EHCI/OHCI bindings: PHYs are moved from the sub-nodes to a standard array
under the 'phys' property. Such solution has been suggested by Måns
Rullgård in the following thread: https://lkml.org/lkml/2019/5/13/228
To keep everything working during the transitional time, the changes has
been split into 2 steps. First the changes to Exynos OHCI and EHCI
drivers have to be merged, then in the next kernel release the DTS can be
finally updated to the new bindings.
This patchset has been tested on various Exynos boards with different
USB host controller configurations (Odroids family: X2, U3, XU3).
Best regards
Marek Szyprowski
Samsung R&D Institute Poland
Changelog:
v2:
- rearranged the code as suggested by Måns Rullgård, kept support for
legacy bindings the Exynos EHCI/OHCI drivers
v1: https://patchwork.kernel.org/cover/10953495/
- initial version
Patch summary:
Marek Szyprowski (3):
dt-bindings: switch Exynos EHCI/OHCI bindings to use array of generic
PHYs
usb: exynos: add support for getting PHYs from the standard dt array
ARM: dts: exynos: Use standard arrays of generic PHYs for EHCI/OHCI
devices
.../devicetree/bindings/usb/exynos-usb.txt | 41 +++++++------------
arch/arm/boot/dts/exynos4.dtsi | 28 ++-----------
.../boot/dts/exynos4210-universal_c210.dts | 8 +---
arch/arm/boot/dts/exynos4412-itop-elite.dts | 9 +---
arch/arm/boot/dts/exynos4412-odroidu3.dts | 8 +---
arch/arm/boot/dts/exynos4412-odroidx.dts | 5 +--
arch/arm/boot/dts/exynos4412-origen.dts | 9 +---
arch/arm/boot/dts/exynos5250.dtsi | 16 ++------
arch/arm/boot/dts/exynos54xx.dtsi | 18 ++------
drivers/usb/host/ehci-exynos.c | 23 +++++++++--
drivers/usb/host/ohci-exynos.c | 23 +++++++++--
11 files changed, 74 insertions(+), 114 deletions(-)
--
2.17.1
^ 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