Devicetree
 help / color / mirror / Atom feed
* [PATCH v6 1/3] drm/panel: simple: Add ability to override typical timing
From: Douglas Anderson @ 2019-07-11 20:34 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Eric Anholt,
	David Airlie, Sam Ravnborg, Jeffy Chen, Doug Anderson,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Boris Brezillon,
	Laurent Pinchart, Daniel Vetter, Enric Balletbo i Serra,
	Stéphane Marchesin, Ezequiel Garcia,
	mka-F7+t8E8rja9g9hUCZPvPmw
In-Reply-To: <20190711203455.125667-1-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

From: Sean Paul <seanpaul@chromium.org>

This patch adds the ability to override the typical display timing for a
given panel. This is useful for devices which have timing constraints
that do not apply across the entire display driver (eg: to avoid
crosstalk between panel and digitizer on certain laptops). The rules are
as follows:

- panel must not specify fixed mode (since the override mode will
  either be the same as the fixed mode, or we'll be unable to
  check the bounds of the overried)
- panel must specify at least one display_timing range which will be
  used to ensure the override mode fits within its bounds

Changes in v2:
 - Parse the full display-timings node (using the native-mode) (Rob)
Changes in v3:
 - No longer parse display-timings subnode, use panel-timing (Rob)
Changes in v4:
 - Don't add mode from timing if override was specified (Thierry)
 - Add warning if timing and fixed mode was specified (Thierry)
 - Don't add fixed mode if timing was specified (Thierry)
 - Refactor/rename a bit to avoid extra indentation from "if" tests
 - i should be unsigned (Thierry)
 - Add annoying WARN_ONs for some cases (Thierry)
 - Simplify 'No display_timing found' handling (Thierry)
 - Rename to panel_simple_parse_override_mode() (Thierry)
Changes in v5:
 - Added Heiko's Tested-by
Changes in v6:
 - Rebased to drm-misc next
 - Added tags

Cc: Doug Anderson <dianders@chromium.org>
Cc: Eric Anholt <eric@anholt.net>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Stéphane Marchesin <marcheu@chromium.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Acked-by: Thierry Reding <thierry.reding@gmail.com>
---

 drivers/gpu/drm/panel/panel-simple.c | 109 +++++++++++++++++++++++++--
 1 file changed, 104 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index af6bf5611b4e..1bee197821ef 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -30,6 +30,7 @@
 #include <linux/regulator/consumer.h>
 
 #include <video/display_timing.h>
+#include <video/of_display_timing.h>
 #include <video/videomode.h>
 
 #include <drm/drm_crtc.h>
@@ -92,6 +93,8 @@ struct panel_simple {
 	struct i2c_adapter *ddc;
 
 	struct gpio_desc *enable_gpio;
+
+	struct drm_display_mode override_mode;
 };
 
 static inline struct panel_simple *to_panel_simple(struct drm_panel *panel)
@@ -99,16 +102,13 @@ static inline struct panel_simple *to_panel_simple(struct drm_panel *panel)
 	return container_of(panel, struct panel_simple, base);
 }
 
-static int panel_simple_get_fixed_modes(struct panel_simple *panel)
+static unsigned int panel_simple_get_timings_modes(struct panel_simple *panel)
 {
 	struct drm_connector *connector = panel->base.connector;
 	struct drm_device *drm = panel->base.drm;
 	struct drm_display_mode *mode;
 	unsigned int i, num = 0;
 
-	if (!panel->desc)
-		return 0;
-
 	for (i = 0; i < panel->desc->num_timings; i++) {
 		const struct display_timing *dt = &panel->desc->timings[i];
 		struct videomode vm;
@@ -132,6 +132,16 @@ static int panel_simple_get_fixed_modes(struct panel_simple *panel)
 		num++;
 	}
 
+	return num;
+}
+
+static unsigned int panel_simple_get_fixed_modes(struct panel_simple *panel)
+{
+	struct drm_connector *connector = panel->base.connector;
+	struct drm_device *drm = panel->base.drm;
+	struct drm_display_mode *mode;
+	unsigned int i, num = 0;
+
 	for (i = 0; i < panel->desc->num_modes; i++) {
 		const struct drm_display_mode *m = &panel->desc->modes[i];
 
@@ -153,6 +163,44 @@ static int panel_simple_get_fixed_modes(struct panel_simple *panel)
 		num++;
 	}
 
+	return num;
+}
+
+static int panel_simple_get_non_edid_modes(struct panel_simple *panel)
+{
+	struct drm_connector *connector = panel->base.connector;
+	struct drm_device *drm = panel->base.drm;
+	struct drm_display_mode *mode;
+	bool has_override = panel->override_mode.type;
+	unsigned int num = 0;
+
+	if (!panel->desc)
+		return 0;
+
+	if (has_override) {
+		mode = drm_mode_duplicate(drm, &panel->override_mode);
+		if (mode) {
+			drm_mode_probed_add(connector, mode);
+			num = 1;
+		} else {
+			dev_err(drm->dev, "failed to add override mode\n");
+		}
+	}
+
+	/* Only add timings if override was not there or failed to validate */
+	if (num == 0 && panel->desc->num_timings)
+		num = panel_simple_get_timings_modes(panel);
+
+	/*
+	 * Only add fixed modes if timings/override added no mode.
+	 *
+	 * We should only ever have either the display timings specified
+	 * or a fixed mode. Anything else is rather bogus.
+	 */
+	WARN_ON(panel->desc->num_timings && panel->desc->num_modes);
+	if (num == 0)
+		num = panel_simple_get_fixed_modes(panel);
+
 	connector->display_info.bpc = panel->desc->bpc;
 	connector->display_info.width_mm = panel->desc->size.width;
 	connector->display_info.height_mm = panel->desc->size.height;
@@ -269,7 +317,7 @@ static int panel_simple_get_modes(struct drm_panel *panel)
 	}
 
 	/* add hard-coded panel modes */
-	num += panel_simple_get_fixed_modes(p);
+	num += panel_simple_get_non_edid_modes(p);
 
 	return num;
 }
@@ -300,10 +348,58 @@ static const struct drm_panel_funcs panel_simple_funcs = {
 	.get_timings = panel_simple_get_timings,
 };
 
+#define PANEL_SIMPLE_BOUNDS_CHECK(to_check, bounds, field) \
+	(to_check->field.typ >= bounds->field.min && \
+	 to_check->field.typ <= bounds->field.max)
+static void panel_simple_parse_override_mode(struct device *dev,
+					     struct panel_simple *panel,
+					     const struct display_timing *ot)
+{
+	const struct panel_desc *desc = panel->desc;
+	struct videomode vm;
+	unsigned int i;
+
+	if (WARN_ON(desc->num_modes)) {
+		dev_err(dev, "Reject override mode: panel has a fixed mode\n");
+		return;
+	}
+	if (WARN_ON(!desc->num_timings)) {
+		dev_err(dev, "Reject override mode: no timings specified\n");
+		return;
+	}
+
+	for (i = 0; i < panel->desc->num_timings; i++) {
+		const struct display_timing *dt = &panel->desc->timings[i];
+
+		if (!PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hactive) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hfront_porch) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hback_porch) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hsync_len) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vactive) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vfront_porch) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vback_porch) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vsync_len))
+			continue;
+
+		if (ot->flags != dt->flags)
+			continue;
+
+		videomode_from_timing(ot, &vm);
+		drm_display_mode_from_videomode(&vm, &panel->override_mode);
+		panel->override_mode.type |= DRM_MODE_TYPE_DRIVER |
+					     DRM_MODE_TYPE_PREFERRED;
+		break;
+	}
+
+	if (WARN_ON(!panel->override_mode.type))
+		dev_err(dev, "Reject override mode: No display_timing found\n");
+}
+
 static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 {
 	struct device_node *backlight, *ddc;
 	struct panel_simple *panel;
+	struct display_timing dt;
 	int err;
 
 	panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL);
@@ -349,6 +445,9 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 		}
 	}
 
+	if (!of_get_display_timing(dev->of_node, "panel-timing", &dt))
+		panel_simple_parse_override_mode(dev, panel, &dt);
+
 	drm_panel_init(&panel->base);
 	panel->base.dev = dev;
 	panel->base.funcs = &panel_simple_funcs;
-- 
2.22.0.410.gd8fdbe21b5-goog


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related

* Re: [PATCH v5 2/7] drm/panel: simple: Add ability to override typical timing
From: Sam Ravnborg @ 2019-07-11 19:38 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Thierry Reding, Heiko Stuebner, Sean Paul, devicetree,
	Rob Herring, David Airlie, Jeffy Chen, dri-devel, LKML,
	open list:ARM/Rockchip SoC..., Boris Brezillon, Laurent Pinchart,
	Enric Balletbò, Stéphane Marchesin, Ezequiel Garcia,
	Matthias Kaehlcke
In-Reply-To: <CAD=FV=XnDTKkscdCwFE1137aX6pTtv=5zqXf=yqcnchpZpt5_Q@mail.gmail.com>

Hi Doug.

> > > > > @@ -91,6 +92,8 @@ struct panel_simple {
> > > > >       struct i2c_adapter *ddc;
> > > > >
> > > > >       struct gpio_desc *enable_gpio;
> > > > > +
> > > > > +     struct drm_display_mode override_mode;
> > > > I fail to see where this poiter is assigned.
> > >
> > > In panel_simple_parse_override_mode().  Specifically:
> > >
> > > drm_display_mode_from_videomode(&vm, &panel->override_mode);
> >
> > The above code-snippet is only called in the panel has specified display
> > timings using display_timings - it is not called when display_mode is
> > used.
> > So override_mode is only assigned in some cases and not all cases.
> > This needs to be fixed so we do not reference override_mode unless
> > it is set.
> 
> I'm afraid I'm not following you here.

> 
> * override_mode is a structure that's directly part of "struct panel_simple".
I had somehow confused myself to think this was a pointer.
And you are right that override_mode is properly initialized when the
structure is allocated.

Sorry for not reading the code and your replies carefully enough.

	Sam

^ permalink raw reply

* Re: [PATCH v5 0/7] drm/panel: simple: Add mode support to devicetree
From: Sam Ravnborg @ 2019-07-11 19:35 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Heiko Stübner, Thierry Reding, Sean Paul, Mark Rutland,
	devicetree, Dmitry Torokhov, Rob Herring, David Airlie,
	Viresh Kumar, Brian Norris, dri-devel, LKML,
	open list:ARM/Rockchip SoC..., Boris Brezillon, Klaus Goger,
	Laurent Pinchart, Enric Balletbò
In-Reply-To: <CAD=FV=U3Wj8vaZcQLmkfX6zgjVFEra+GrHMH3OCs5QQ_-tM4hw@mail.gmail.com>

Hi Dough.

> > So Thierry was able to look at the patches yesterday it seems and has Acked
> > all the relevant ones. As a drm-misc-contributor I could also apply them
> > myself, but now don't want to preempt any additional comments you might
> > have ;-) . So I guess my question would be if you still want to do a review
> > or if I should apply them.
> 
> Hopefully you saw, but I responded to all of your review feedback.  In
> the end, I thought it'd be OK to land the series as-is and I can fix
> up nits in a follow-up series, though I'm waiting for your responses
> to a couple questions first.
> 
> It would be ideal if you could confirm that you're OK with this plan
> even if you don't have time to respond in detail to my emails yet.  I
> think Heiko can land them all through the appropriate channels since
> the patches have all the proper Acks.
My main concern was the bug cocerning override_mode - which turned out to
be me confused.
There is one part about flags that does not yet makes sense but
we can fix it later.

Please resend a series that applies to drm-misc-next so
we can land this.

	Sam

^ permalink raw reply

* Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
From: Michael Nazzareno Trimarchi @ 2019-07-11 19:34 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Jagan Teki, David Airlie, Daniel Vetter, Chen-Yu Tsai,
	Michael Turquette, Rob Herring, Mark Rutland, linux-arm-kernel,
	linux-kernel, linux-clk, dri-devel, devicetree, linux-amarula,
	linux-sunxi
In-Reply-To: <CAOf5uw=3fiMuhcj3kDtCaGNTsxHKRrYb79MXZ+yUZtmf0jU10A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi

On Thu, Jul 11, 2019 at 7:43 PM Michael Nazzareno Trimarchi
<michael-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org> wrote:
>
> Hi Maxime
>
> On Thu, Jul 11, 2019 at 2:23 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> >
> > On Fri, Jul 05, 2019 at 07:52:27PM +0200, Michael Nazzareno Trimarchi wrote:
> > > On Wed, Jul 3, 2019 at 1:49 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > > >
> > > > On Tue, Jun 25, 2019 at 09:00:36PM +0530, Jagan Teki wrote:
> > > > > On Tue, Jun 25, 2019 at 8:19 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > > > > >
> > > > > > On Thu, Jun 20, 2019 at 11:57:44PM +0530, Jagan Teki wrote:
> > > > > > > On Fri, Jun 14, 2019 at 7:54 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > > > > > > >
> > > > > > > > On Wed, Jun 05, 2019 at 01:03:16PM +0530, Jagan Teki wrote:
> > > > > > > > > On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi,
> > > > > > > > > >
> > > > > > > > > > I've reordered the mail a bit to work on chunks
> > > > > > > > > >
> > > > > > > > > > On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote:
> > > > > > > > > > > > I wish it was in your commit log in the first place, instead of having
> > > > > > > > > > > > to exchange multiple mails over this.
> > > > > > > > > > > >
> > > > > > > > > > > > However, I don't think that's quite true, and it might be a bug in
> > > > > > > > > > > > Allwinner's implementation (or rather something quite confusing).
> > > > > > > > > > > >
> > > > > > > > > > > > You're right that the lcd_rate and pll_rate seem to be generated from
> > > > > > > > > > > > the pixel clock, and it indeed looks like the ratio between the pixel
> > > > > > > > > > > > clock and the TCON dotclock is defined through the number of bits per
> > > > > > > > > > > > lanes.
> > > > > > > > > > > >
> > > > > > > > > > > > However, in this case, dsi_rate is actually the same than lcd_rate,
> > > > > > > > > > > > since pll_rate is going to be divided by dsi_div:
> > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L791
> > > > > > > > > > > >
> > > > > > > > > > > > Since lcd_div is 1, it also means that in this case, dsi_rate ==
> > > > > > > > > > > > dclk_rate.
> > > > > > > > > > > >
> > > > > > > > > > > > The DSI module clock however, is always set to 148.5 MHz. Indeed, if
> > > > > > > > > > > > we look at:
> > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L804
> > > > > > > > > > > >
> > > > > > > > > > > > We can see that the rate in clk_info is used if it's different than
> > > > > > > > > > > > 0. This is filled by disp_al_lcd_get_clk_info, which, in the case of a
> > > > > > > > > > > > DSI panel, will hardcode it to 148.5 MHz:
> > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/disp_al.c#L164
> > > > > > > > > > >
> > > > > > > > > > > Let me explain, something more.
> > > > > > > > > > >
> > > > > > > > > > > According to bsp there are clk_info.tcon_div which I will explain below.
> > > > > > > > > > > clk_info.dsi_div which is dynamic and it depends on bpp/lanes, so it
> > > > > > > > > > > is 6 for 24bpp and 4 lanes devices.
> > > > > > > > > > >
> > > > > > > > > > > PLL rate here depends on dsi_div (not tcon_div)
> > > > > > > > > > >
> > > > > > > > > > > Code here
> > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L784
> > > > > > > > > > >
> > > > > > > > > > > is computing the actual set rate, which depends on dsi_rate.
> > > > > > > > > > >
> > > > > > > > > > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > > > > > > > > > dsi_rate = pll_rate / clk_info.dsi_div;
> > > > > > > > > > >
> > > > > > > > > > > Say if the dclk_rate 148MHz then the dsi_rate is 888MHz which set rate
> > > > > > > > > > > for above link you mentioned.
> > > > > > > > > > >
> > > > > > > > > > > Here are the evidence with some prints.
> > > > > > > > > > >
> > > > > > > > > > > https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043
> > > > > > > > > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > > > > > > > > >
> > > > > > > > > > Ok, so we agree up to this point, and the prints confirm that the
> > > > > > > > > > analysis above is the right one.
> > > > > > > > > >
> > > > > > > > > > > > So, the DSI clock is set to this here:
> > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L805
> > > > > > > > > >
> > > > > > > > > > Your patch doesn't address that, so let's leave that one alone.
> > > > > > > > >
> > > > > > > > > Basically this is final pll set rate when sun4i_dotclock.c called the
> > > > > > > > > desired rate with ccu_nkm.c so it ended the final rate with parent as
> > > > > > > > > Line 8 of
> > > > > > > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > > > > > > >
> > > > > > > > If that's important to the driver, it should be set explicitly then,
> > > > > > > > and not work by accident.
> > > > > > > >
> > > > > > > > > > > > The TCON *module* clock (the one in the clock controller) has been set
> > > > > > > > > > > > to lcd_rate (so the pixel clock times the number of bits per lane) here:
> > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L800
> > > > > > > > > > > >
> > > > > > > > > > > > And the PLL has been set to the same rate here:
> > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L794
> > > > > > > > > > > >
> > > > > > > > > > > > Let's take a step back now: that function we were looking at,
> > > > > > > > > > > > lcd_clk_config, is called by lcd_clk_enable, which is in turn called
> > > > > > > > > > > > by disp_lcd_enable here:
> > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L1328
> > > > > > > > > > > >
> > > > > > > > > > > > The next function being called is disp_al_lcd_cfg, and that function
> > > > > > > > > > > > will hardcode the TCON dotclock divider to 4, here:
> > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/disp_al.c#L240
> > > > > > > > > > >
> > > > > > > > > > > tcon_div from BSP point-of-view of there are two variants
> > > > > > > > > > > 00) clk_info.tcon_div which is 4 and same is set the divider position
> > > > > > > > > > > in SUN4I_TCON0_DCLK_REG (like above link refer)
> > > > > > > > > > > 01) tcon_div which is 4 and used for edge timings computation
> > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L12
> > > > > > > > > > >
> > > > > > > > > > > The real reason for 01) is again 4 is they set the divider to 4 in 00)
> > > > > > > > > > > which is technically wrong because the dividers which used during
> > > > > > > > > > > dotclock in above (dsi_div) should be used here as well. Since there
> > > > > > > > > > > is no dynamic way of doing this BSP hard-coding these values.
> > > > > > > > > > >
> > > > > > > > > > > Patches 5,6,7 on this series doing this
> > > > > > > > > > > https://patchwork.freedesktop.org/series/60847/
> > > > > > > > > > >
> > > > > > > > > > > Hope this explanation helps?
> > > > > > > > > >
> > > > > > > > > > It doesn't.
> > > > > > > > > >
> > > > > > > > > > The clock tree is this one:
> > > > > > > > > >
> > > > > > > > > > PLL(s) -> TCON module clock -> TCON dotclock.
> > > > > > > > > >
> > > > > > > > > > The links I mentioned above show that the clock set to lcd_rate is the
> > > > > > > > > > TCON module clocks (and it should be the one taking the bpp and lanes
> > > > > > > > > > into account), while the TCON dotclock uses a fixed divider of 4.
> > > > > > > > >
> > > > > > > > > Sorry, I can argue much other-than giving some code snips, according to [1]
> > > > > > > > >
> > > > > > > > > 00) Line 785, 786 with dclk_rate 148000000
> > > > > > > > >
> > > > > > > > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > > > > > > > pll_rate = lcd_rate * clk_info.lcd_div;
> > > > > > > > >
> > > > > > > > > Since dsi_div is 6 (bpp/lanes), lcd_div 1
> > > > > > > > >
> > > > > > > > > lcd_rate = 888000000, pll_rate = 888000000
> > > > > > > > >
> > > > > > > > > 01)  Line 801, 804 are final rates computed as per clock driver (say
> > > > > > > > > ccu_nkm in mainline)
> > > > > > > > >
> > > > > > > > > lcd_rate_set=891000000
> > > > > > > > >
> > > > > > > > > As per your comments if it would be 4 then the desired numbers are
> > > > > > > > > would be 592000000 not 888000000.
> > > > > > > > >
> > > > > > > > > This is what I'm trying to say in all mails, and same as verified with
> > > > > > > > > 2-lanes devices as well where the dsi_div is 12 so the final rate is
> > > > > > > > > 290MHz * 12
> > > > > > > >
> > > > > > > > In the code you sent, you're forcing a divider on the internal TCON
> > > > > > > > clock, while that one is fixed in the BSP.
> > > > > > > >
> > > > > > > > There's indeed the bpp / lanes divider, but it's used in the *parent*
> > > > > > > > clock of the one you're changing.
> > > > > > > >
> > > > > > > > And the dsi0_clk clock you pointed out in the code snippet is yet
> > > > > > > > another clock, the MIPI DSI module clock.
> > > > > > >
> > > > > > > Correct, look like I refereed wrong reference in the above mail. sorry
> > > > > > > for the noise.
> > > > > > >
> > > > > > > Actually I'm trying to explain about pll_rate here which indeed
> > > > > > > depends on dsi.div
> > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L786
> > > > > > >
> > > > > > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > > > > > pll_rate = lcd_rate * clk_info.lcd_div;
> > > > > > >
> > > > > > > Say
> > > > > > >
> > > > > > > 1) For 148MHz dclk_rate with dsi_div is 6 (24/4) lcd_div is 1 which
> > > > > > > resulting pll_rate is 888MHz.
> > > > > > >
> > > > > > > 2) For 30MHz dclk_rate with 4 lane and 24 RGB the resulting pll_rate is 180MHz
> > > > > > >
> > > > > > > 3) For 27.5MHz dclk_rate with 2 lane and 24 RGB the resulting pll_rate is 330MHz
> > > > > > >
> > > > > > > Here is the few more logs in code, for case 2)
> > > > > > >
> > > > > > > [    1.920441] sun4i_dclk_round_rate: min_div = 6 max_div = 6, rate = 30000000
> > > > > > > [    1.920505] ideal = 180000000, rounded = 178200000
> > > > > > > [    1.920509] sun4i_dclk_round_rate: div = 6 rate = 29700000
> > > > > > > [    1.920514] sun4i_dclk_round_rate: min_div = 6 max_div = 6, rate = 30000000
> > > > > > > [    1.920532] ideal = 1800ls and one DSI-RGB bridge. All of them do use
> > > > > PLL_MIPI (pll_rate) and it indeed depends on bpp/lanes
> > > > >00000, rounded = 178200000
> > > > > > > [    1.920535] sun4i_dclk_round_rate: div = 6 rate = 29700000
> > > > > > > [    1.920572] sun4i_dclk_recalc_rate: val = 1, rate = 178200000
> > > > > > > [    1.920576] sun4i_dclk_recalc_rate: val = 1, rate = 178200000
> > > > > > > [    1.920597] rate = 178200000
> > > > > > > [    1.920599] parent_rate = 297000000
> > > > > > > [    1.920602] reg = 0x90c00000
> > > > > > > [    1.920605] _nkm.n = 3, nkm->n.offset = 0x1, nkm->n.shift = 8
> > > > > > > [    1.920609] _nkm.k = 2, nkm->k.offset = 0x1, nkm->k.shift = 4
> > > > > > > [    1.920612] _nkm.m = 10, nkm->m.offset = 0x1, nkm->m.shift = 0
> > > > > > > [    1.920958] sun4i_dclk_set_rate div 6
> > > > > > > [    1.920966] sun4i_dclk_recalc_rate: val = 6, rate = 29700000
> > > > > > >
> > > > > > > and clk_summary:
> > > > > > >
> > > > > > >     pll-video0                        1        1        1   297000000
> > > > > > >         0     0  50000
> > > > > > >        hdmi                           0        0        0   297000000
> > > > > > >         0     0  50000
> > > > > > >        tcon1                          0        0        0   297000000
> > > > > > >         0     0  50000
> > > > > > >        pll-mipi                       1        1        1   178200000
> > > > > > >         0     0  50000
> > > > > > >           tcon0                       2        2        1   178200000
> > > > > > >         0     0  50000
> > > > > > >              tcon-pixel-clock         1        1        1    29700000
> > > > > > >         0     0  50000
> > > > > > >        pll-video0-2x                  0        0        0   594000000
> > > > > > >         0     0  50000
> > > > > >
> > > > > > This discussion is going nowhere. I'm telling you that your patch
> > > > > > doesn't apply the divider you want on the proper clock, and you're
> > > > > > replying that indeed, you're applying it on the wrong clock.
> > > > > >
> > > > > > It might work by accident in your case, but the board I have here
> > > > > > clearly indicates otherwise, so there's two possible way out here:
> > > > > >
> > > > > >   - Either you apply that divider to the TCON *module* clock, and not
> > > > > >     the dclk
> > > > > >
> > > > > >   - Or you point to somewhere in the allwinner code where the bpp /
> > > > > >     lanes divider is used for the dclk divider.
> > > > >
> > > > > I don't know how to proceed further on this, as you say it might work
> > > > > in accident but I have tested this in A33, A64 and R40 with 4
> > > > > different DSI panels and one DSI-RGB bridge. All of them do use
> > > > > PLL_MIPI (pll_rate) and it indeed depends on bpp/lanes
> > > > >
> > > > > 4-lane, 24-bit: Novatek NT35596 panel
> > > > > 4-lane, 24-bit: Feiyang, FY07024di26a30d panel
> > > > > 4-lane, 24-bit: Bananapi-s070wv20 panel
> > > > > 2-lane, 24-bit: Techstar,ts8550b panel
> > > > >
> > > > > and
> > > > >
> > > > > 4-lane, 24-bit, ICN6211 DSI-to-RGB bridge panel
> > > > >
> > > > > All above listed panels and bridges are working as per BSP and do
> > > > > follow bpp/lanes and for DIVIDER 4 no panel is working.
> > > >
> > > > Look. I'm not saying that there's no issue, I'm saying that your
> > > > patch, applied to the clock you're applying it to, doesn't make sense
> > > > and isn't what the BSP does.
> > >
> > > tcon-pixel clock is the rate that you want to achive on display side
> > > and if you have 4 lanes 32bit or lanes and different bit number that
> > > you need to have a clock that is able to put outside bits and speed
> > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > > the display.
> >
> > So this is what the issue is then?
> >
> > This one does make sense, and you should just change the rate in the
> > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> >
> > I'm still wondering why that hasn't been brought up in either the
> > discussion or the commit log before though.
> >
> Something like this?
>
> drivers/gpu/drm/sun4i/sun4i_tcon.c     | 20 +++++++++++---------
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
>  2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 64c43ee6bd92..42560d5c327c 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> drm_display_mode *mode,
>  }
>
>  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> -                                       const struct drm_display_mode *mode)
> +                                       const struct drm_display_mode *mode,
> +                                       u32 tcon_mul)
>  {
>         /* Configure the dot clock */
> -       clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> +       clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
>
>         /* Set the resolution */
>         regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> sun4i_tcon *tcon,
>         u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
>         u8 lanes = device->lanes;
>         u32 block_space, start_delay;
> -       u32 tcon_div;
> +       u32 tcon_div, tcon_mul;
>
> -       tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> -       tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> +       tcon->dclk_min_div = 4;
> +       tcon->dclk_max_div = 127;
>
> -       sun4i_tcon0_mode_set_common(tcon, mode);
> +       tcon_mul = bpp / lanes;
> +       sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
>

Maybe wrong clock ;) but is this your idea?

Michael
>         /* Set dithering if needed */
>         sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
> @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> sun4i_tcon *tcon,
>          */
>         regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, &tcon_div);
>         tcon_div &= GENMASK(6, 0);
> -       block_space = mode->htotal * bpp / (tcon_div * lanes);
> +       block_space = mode->htotal * tcon_div * tcon_mul;
>         block_space -= mode->hdisplay + 40;
>
>         regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> sun4i_tcon *tcon,
>
>         tcon->dclk_min_div = 7;
>         tcon->dclk_max_div = 7;
> -       sun4i_tcon0_mode_set_common(tcon, mode);
> +       sun4i_tcon0_mode_set_common(tcon, mode, 1);
>
>         /* Set dithering if needed */
>         sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
> @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> sun4i_tcon *tcon,
>
>         tcon->dclk_min_div = 6;
>         tcon->dclk_max_div = 127;
> -       sun4i_tcon0_mode_set_common(tcon, mode);
> +       sun4i_tcon0_mode_set_common(tcon, mode, 1);
>
>         /* Set dithering if needed */
>         sun4i_tcon0_mode_set_dithering(tcon, connector);
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> index 5c3ad5be0690..a07090579f84 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> @@ -13,8 +13,6 @@
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_mipi_dsi.h>
>
> -#define SUN6I_DSI_TCON_DIV     4
> -
>  struct sun6i_dsi {
>         struct drm_connector    connector;
>         struct drm_encoder      encoder;
>
>
> > > Now in burst mode I don't know how should work the calculation of
> > > the clock for the require bandwidth and even I understand your
> > > comment I would like to have your clock tree after you boot on the
> > > display side and if it is possible I want to assemble a kit like you
> > > have.
> >
> > The setup is probably going to be a bit difficult to reproduce, it's a
> > prototype that I have that can't really be found anywhere. Jagan asked
> > me on IRC for the reference, and he found the part, but it's unclear
> > to me if it can be easily adapted to a common board.
> >
> > However, I'm not even sure we need this. I'll test the next version
> > and let you know if it works.
> >
> > Maxime
> >
> > --
> > Maxime Ripard, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
>
>
>
> --
> | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
> | COO  -  Founder                                      Cruquiuskade 47 |
> | +31(0)851119172                                 Amsterdam 1018 AM NL |
> |                  [`as] http://www.amarulasolutions.com               |



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

^ permalink raw reply

* Re: [PATCH v8 3/5] mtd: Add support for HyperBus memory devices
From: Sergei Shtylyov @ 2019-07-11 19:26 UTC (permalink / raw)
  To: Vignesh Raghavendra, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Rob Herring
  Cc: linux-mtd, Miquel Raynal, devicetree, Mason Yang,
	linux-arm-kernel, linux-kernel, Tokunori Ikegami
In-Reply-To: <20190625075746.10439-4-vigneshr@ti.com>

Hello!

On 06/25/2019 10:57 AM, Vignesh Raghavendra wrote:

> Cypress' HyperBus is Low Signal Count, High Performance Double Data Rate
> Bus interface between a host system master and one or more slave
> interfaces. HyperBus is used to connect microprocessor, microcontroller,
> or ASIC devices with random access NOR flash memory (called HyperFlash)
> or self refresh DRAM (called HyperRAM).
> 
> Its a 8-bit data bus (DQ[7:0]) with  Read-Write Data Strobe (RWDS)
> signal and either Single-ended clock(3.0V parts) or Differential clock
> (1.8V parts). It uses ChipSelect lines to select b/w multiple slaves.
> At bus level, it follows a separate protocol described in HyperBus
> specification[1].
> 
> HyperFlash follows CFI AMD/Fujitsu Extended Command Set (0x0002) similar
> to that of existing parallel NORs. Since HyperBus is x8 DDR bus,
> its equivalent to x16 parallel NOR flash with respect to bits per clock
> cycle. But HyperBus operates at >166MHz frequencies.
> HyperRAM provides direct random read/write access to flash memory
> array.
> 
> But, HyperBus memory controllers seem to abstract implementation details
> and expose a simple MMIO interface to access connected flash.
> 
> Add support for registering HyperFlash devices with MTD framework. MTD
> maps framework along with CFI chip support framework are used to support
> communicating with flash.
> 
> Framework is modelled along the lines of spi-nor framework. HyperBus
> memory controller (HBMC) drivers calls hyperbus_register_device() to
> register a single HyperFlash device. HyperFlash core parses MMIO access
> information from DT, sets up the map_info struct, probes CFI flash and
> registers it with MTD framework.
> 
> Some HBMC masters need calibration/training sequence[3] to be carried
> out, in order for DLL inside the controller to lock, by reading a known
> string/pattern. This is done by repeatedly reading CFI Query
> Identification String. Calibration needs to be done before trying to detect
> flash as part of CFI flash probe.
> 
> HyperRAM is not supported at the moment.
> 
> HyperBus specification can be found at[1]
> HyperFlash datasheet can be found at[2]
> 
> [1] https://www.cypress.com/file/213356/download
> [2] https://www.cypress.com/file/213346/download
> [3] http://www.ti.com/lit/ug/spruid7b/spruid7b.pdf
>     Table 12-5741. HyperFlash Access Sequence
> 
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
[...]

> diff --git a/drivers/mtd/hyperbus/hyperbus-core.c b/drivers/mtd/hyperbus/hyperbus-core.c
> new file mode 100644
> index 000000000000..63a9e64895bc
> --- /dev/null
> +++ b/drivers/mtd/hyperbus/hyperbus-core.c
> @@ -0,0 +1,154 @@
[...]
> +int hyperbus_register_device(struct hyperbus_device *hbdev)
> +{
[...]
> +	map->name = dev_name(dev);
> +	map->bankwidth = 2;

   I think this should really be 1, judging on the comment to that field (and on
Cogent's own RPC-IF HF driver).

> +	map->device_node = np;

[...]

MBR, Sergei

^ permalink raw reply

* Re: [PATCH v5 2/7] drm/panel: simple: Add ability to override typical timing
From: Sean Paul @ 2019-07-11 19:16 UTC (permalink / raw)
  To: Doug Anderson
  Cc: devicetree, David Airlie, Ezequiel Garcia, Jeffy Chen, LKML,
	dri-devel, open list:ARM/Rockchip SoC..., Rob Herring,
	Thierry Reding, Laurent Pinchart, Boris Brezillon,
	Enric Balletbò, Stéphane Marchesin, Sam Ravnborg,
	Matthias Kaehlcke
In-Reply-To: <CAD=FV=VdkPLwyGhSnrHCcduQAPwby35Mqhk_r=O595bMoMT=6w@mail.gmail.com>

On Wed, Jul 10, 2019 at 6:56 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Jul 8, 2019 at 10:56 AM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > On Mon, Jul 01, 2019 at 09:39:06AM -0700, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Sun, Jun 30, 2019 at 1:55 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> > > >
> > > > Hi Douglas.
> > > >
> > > > > > +
> > > > > > +   /* Only add timings if override was not there or failed to validate */
> > > > > > +   if (num == 0 && panel->desc->num_timings)
> > > > > > +           num = panel_simple_get_timings_modes(panel);
> > > > > > +
> > > > > > +   /*
> > > > > > +    * Only add fixed modes if timings/override added no mode.
> > > > >
> > > > > This part I fail to understand.
> > > > > If we have a panel where we in panel-simple have specified the timings,
> > > > > and done so using display_timing so with proper {min, typ, max} then it
> > > > > should be perfectly legal to specify a more precise variant in the DT
> > > > > file.
> > > > > Or what did I miss here?
> > > >
> > > > Got it now.
> > > > If display_mode is used for timings this is what you call "fixed mode".
> > > > Hmm, if I got confused someone else may also be confused by this naming.
> > >
> > > The name "fixed mode" comes from the old code, though I guess in the
> > > old code it used to refer to a mode that came from either the
> > > display_timing or the display_mode.
> > >
> > > How about if I call it "panel_simple_get_from_fixed_display_mode"?
> > > ...or if you have another suggestion feel free to chime in.
> > What we really want to distingush here is the use of display_mode
> > and display_timings (if I got the names right).
> > That display_mode specify a fixed timing and display_timing specify
> > a valid range is something in the semantics of the two types.
> > So naming that refer to display_mode versus display_timing will make the
> > code simpler to understand. and then a nice comment that when
> > display_mode
> > is used one looses the possibility to use override_mode.
> > That would be fine to have in the struct in the driver.
>
> OK, I can change the names here and try to find a good place to add a comment.
>
>
> > > NOTE: Since this feedback is minor and this patch has been outstanding
> > > for a while (and is blocking other work), I am assuming that the best
> > > path forward is for Heiko to land this patch with Thierry's Ack and
> > > I'll send a follow-up.  Please yell if you disagree.
> > Let's give the patches a spin more as we have passed the possibility for
> > the current merge window.
>
> Any way I can convince you to change your mind here?  There are no
> functional changes requested so far in your feedback and no bugs--it's
> just a few variable names and comments.  By landing the existing
> patches as-is:
>
> 1. We stop spamming all the people CCed on this whole series (which
> includes device tree patches) that might be interested in the series
> as a whole but aren't interested in details.
>
> 2. We can debate the bikeshed-type issues on their own merit and I
> don't have to debate removing existing Acks / Reviewed-by / Tested-by
> tags as I make changes.
>
> 3. Even if it's not a good time to land the patches right now we know
> that these patches will be ready to land as soon as the window opens.
> As I mentioned earlier these patches are blocking other work [1] and
> landing that patch is actually preventing Matthias from submitting
> another series of patches to add support for rk3288-veyron-tiger and
> rk3288-veyron-fievel.  Certainly I know that upstream doesn't make a
> policy of landing things just to suit the timelines of a downstream
> project, but in this case there seems very little downsides to landing
> the existing patches and taking a later cleanup patch.
>

[sending from my @chromium.org address so any appearance of bias is
explicit :) ]

Agree with Doug here, the naming and casting discussion is pretty
subjective and non-functional. We've got an Ack from Thierry and a
Review from Boris (both seasoned drm_panel'ers), this patch has been
sitting in review for a while. Let's not let the perfect be the enemy
of the good.

Sean

>
> > I am on vacation at the moment and thus slow in responses, but will be back
> > at the home office next week and will be more responsive again.
> >
> >         Sam - who is enjoying the alps in Austria
>
> Hope you have had a great vacation!
>
> [1] https://lkml.kernel.org/r/20190625222629.154619-1-mka@chromium.org
>
> -Doug
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
From: Michael Nazzareno Trimarchi @ 2019-07-11 17:43 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Jagan Teki, David Airlie, Daniel Vetter, Chen-Yu Tsai,
	Michael Turquette, Rob Herring, Mark Rutland, linux-arm-kernel,
	linux-kernel, linux-clk, dri-devel, devicetree, linux-amarula,
	linux-sunxi
In-Reply-To: <20190711100100.cty3s6rs3w27low6@flea>

Hi Maxime

On Thu, Jul 11, 2019 at 2:23 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
>
> On Fri, Jul 05, 2019 at 07:52:27PM +0200, Michael Nazzareno Trimarchi wrote:
> > On Wed, Jul 3, 2019 at 1:49 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > >
> > > On Tue, Jun 25, 2019 at 09:00:36PM +0530, Jagan Teki wrote:
> > > > On Tue, Jun 25, 2019 at 8:19 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > > > >
> > > > > On Thu, Jun 20, 2019 at 11:57:44PM +0530, Jagan Teki wrote:
> > > > > > On Fri, Jun 14, 2019 at 7:54 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > > > > > >
> > > > > > > On Wed, Jun 05, 2019 at 01:03:16PM +0530, Jagan Teki wrote:
> > > > > > > > On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > I've reordered the mail a bit to work on chunks
> > > > > > > > >
> > > > > > > > > On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote:
> > > > > > > > > > > I wish it was in your commit log in the first place, instead of having
> > > > > > > > > > > to exchange multiple mails over this.
> > > > > > > > > > >
> > > > > > > > > > > However, I don't think that's quite true, and it might be a bug in
> > > > > > > > > > > Allwinner's implementation (or rather something quite confusing).
> > > > > > > > > > >
> > > > > > > > > > > You're right that the lcd_rate and pll_rate seem to be generated from
> > > > > > > > > > > the pixel clock, and it indeed looks like the ratio between the pixel
> > > > > > > > > > > clock and the TCON dotclock is defined through the number of bits per
> > > > > > > > > > > lanes.
> > > > > > > > > > >
> > > > > > > > > > > However, in this case, dsi_rate is actually the same than lcd_rate,
> > > > > > > > > > > since pll_rate is going to be divided by dsi_div:
> > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L791
> > > > > > > > > > >
> > > > > > > > > > > Since lcd_div is 1, it also means that in this case, dsi_rate ==
> > > > > > > > > > > dclk_rate.
> > > > > > > > > > >
> > > > > > > > > > > The DSI module clock however, is always set to 148.5 MHz. Indeed, if
> > > > > > > > > > > we look at:
> > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L804
> > > > > > > > > > >
> > > > > > > > > > > We can see that the rate in clk_info is used if it's different than
> > > > > > > > > > > 0. This is filled by disp_al_lcd_get_clk_info, which, in the case of a
> > > > > > > > > > > DSI panel, will hardcode it to 148.5 MHz:
> > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/disp_al.c#L164
> > > > > > > > > >
> > > > > > > > > > Let me explain, something more.
> > > > > > > > > >
> > > > > > > > > > According to bsp there are clk_info.tcon_div which I will explain below.
> > > > > > > > > > clk_info.dsi_div which is dynamic and it depends on bpp/lanes, so it
> > > > > > > > > > is 6 for 24bpp and 4 lanes devices.
> > > > > > > > > >
> > > > > > > > > > PLL rate here depends on dsi_div (not tcon_div)
> > > > > > > > > >
> > > > > > > > > > Code here
> > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L784
> > > > > > > > > >
> > > > > > > > > > is computing the actual set rate, which depends on dsi_rate.
> > > > > > > > > >
> > > > > > > > > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > > > > > > > > dsi_rate = pll_rate / clk_info.dsi_div;
> > > > > > > > > >
> > > > > > > > > > Say if the dclk_rate 148MHz then the dsi_rate is 888MHz which set rate
> > > > > > > > > > for above link you mentioned.
> > > > > > > > > >
> > > > > > > > > > Here are the evidence with some prints.
> > > > > > > > > >
> > > > > > > > > > https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043
> > > > > > > > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > > > > > > > >
> > > > > > > > > Ok, so we agree up to this point, and the prints confirm that the
> > > > > > > > > analysis above is the right one.
> > > > > > > > >
> > > > > > > > > > > So, the DSI clock is set to this here:
> > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L805
> > > > > > > > >
> > > > > > > > > Your patch doesn't address that, so let's leave that one alone.
> > > > > > > >
> > > > > > > > Basically this is final pll set rate when sun4i_dotclock.c called the
> > > > > > > > desired rate with ccu_nkm.c so it ended the final rate with parent as
> > > > > > > > Line 8 of
> > > > > > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > > > > > >
> > > > > > > If that's important to the driver, it should be set explicitly then,
> > > > > > > and not work by accident.
> > > > > > >
> > > > > > > > > > > The TCON *module* clock (the one in the clock controller) has been set
> > > > > > > > > > > to lcd_rate (so the pixel clock times the number of bits per lane) here:
> > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L800
> > > > > > > > > > >
> > > > > > > > > > > And the PLL has been set to the same rate here:
> > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L794
> > > > > > > > > > >
> > > > > > > > > > > Let's take a step back now: that function we were looking at,
> > > > > > > > > > > lcd_clk_config, is called by lcd_clk_enable, which is in turn called
> > > > > > > > > > > by disp_lcd_enable here:
> > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L1328
> > > > > > > > > > >
> > > > > > > > > > > The next function being called is disp_al_lcd_cfg, and that function
> > > > > > > > > > > will hardcode the TCON dotclock divider to 4, here:
> > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/disp_al.c#L240
> > > > > > > > > >
> > > > > > > > > > tcon_div from BSP point-of-view of there are two variants
> > > > > > > > > > 00) clk_info.tcon_div which is 4 and same is set the divider position
> > > > > > > > > > in SUN4I_TCON0_DCLK_REG (like above link refer)
> > > > > > > > > > 01) tcon_div which is 4 and used for edge timings computation
> > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L12
> > > > > > > > > >
> > > > > > > > > > The real reason for 01) is again 4 is they set the divider to 4 in 00)
> > > > > > > > > > which is technically wrong because the dividers which used during
> > > > > > > > > > dotclock in above (dsi_div) should be used here as well. Since there
> > > > > > > > > > is no dynamic way of doing this BSP hard-coding these values.
> > > > > > > > > >
> > > > > > > > > > Patches 5,6,7 on this series doing this
> > > > > > > > > > https://patchwork.freedesktop.org/series/60847/
> > > > > > > > > >
> > > > > > > > > > Hope this explanation helps?
> > > > > > > > >
> > > > > > > > > It doesn't.
> > > > > > > > >
> > > > > > > > > The clock tree is this one:
> > > > > > > > >
> > > > > > > > > PLL(s) -> TCON module clock -> TCON dotclock.
> > > > > > > > >
> > > > > > > > > The links I mentioned above show that the clock set to lcd_rate is the
> > > > > > > > > TCON module clocks (and it should be the one taking the bpp and lanes
> > > > > > > > > into account), while the TCON dotclock uses a fixed divider of 4.
> > > > > > > >
> > > > > > > > Sorry, I can argue much other-than giving some code snips, according to [1]
> > > > > > > >
> > > > > > > > 00) Line 785, 786 with dclk_rate 148000000
> > > > > > > >
> > > > > > > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > > > > > > pll_rate = lcd_rate * clk_info.lcd_div;
> > > > > > > >
> > > > > > > > Since dsi_div is 6 (bpp/lanes), lcd_div 1
> > > > > > > >
> > > > > > > > lcd_rate = 888000000, pll_rate = 888000000
> > > > > > > >
> > > > > > > > 01)  Line 801, 804 are final rates computed as per clock driver (say
> > > > > > > > ccu_nkm in mainline)
> > > > > > > >
> > > > > > > > lcd_rate_set=891000000
> > > > > > > >
> > > > > > > > As per your comments if it would be 4 then the desired numbers are
> > > > > > > > would be 592000000 not 888000000.
> > > > > > > >
> > > > > > > > This is what I'm trying to say in all mails, and same as verified with
> > > > > > > > 2-lanes devices as well where the dsi_div is 12 so the final rate is
> > > > > > > > 290MHz * 12
> > > > > > >
> > > > > > > In the code you sent, you're forcing a divider on the internal TCON
> > > > > > > clock, while that one is fixed in the BSP.
> > > > > > >
> > > > > > > There's indeed the bpp / lanes divider, but it's used in the *parent*
> > > > > > > clock of the one you're changing.
> > > > > > >
> > > > > > > And the dsi0_clk clock you pointed out in the code snippet is yet
> > > > > > > another clock, the MIPI DSI module clock.
> > > > > >
> > > > > > Correct, look like I refereed wrong reference in the above mail. sorry
> > > > > > for the noise.
> > > > > >
> > > > > > Actually I'm trying to explain about pll_rate here which indeed
> > > > > > depends on dsi.div
> > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L786
> > > > > >
> > > > > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > > > > pll_rate = lcd_rate * clk_info.lcd_div;
> > > > > >
> > > > > > Say
> > > > > >
> > > > > > 1) For 148MHz dclk_rate with dsi_div is 6 (24/4) lcd_div is 1 which
> > > > > > resulting pll_rate is 888MHz.
> > > > > >
> > > > > > 2) For 30MHz dclk_rate with 4 lane and 24 RGB the resulting pll_rate is 180MHz
> > > > > >
> > > > > > 3) For 27.5MHz dclk_rate with 2 lane and 24 RGB the resulting pll_rate is 330MHz
> > > > > >
> > > > > > Here is the few more logs in code, for case 2)
> > > > > >
> > > > > > [    1.920441] sun4i_dclk_round_rate: min_div = 6 max_div = 6, rate = 30000000
> > > > > > [    1.920505] ideal = 180000000, rounded = 178200000
> > > > > > [    1.920509] sun4i_dclk_round_rate: div = 6 rate = 29700000
> > > > > > [    1.920514] sun4i_dclk_round_rate: min_div = 6 max_div = 6, rate = 30000000
> > > > > > [    1.920532] ideal = 1800ls and one DSI-RGB bridge. All of them do use
> > > > PLL_MIPI (pll_rate) and it indeed depends on bpp/lanes
> > > >00000, rounded = 178200000
> > > > > > [    1.920535] sun4i_dclk_round_rate: div = 6 rate = 29700000
> > > > > > [    1.920572] sun4i_dclk_recalc_rate: val = 1, rate = 178200000
> > > > > > [    1.920576] sun4i_dclk_recalc_rate: val = 1, rate = 178200000
> > > > > > [    1.920597] rate = 178200000
> > > > > > [    1.920599] parent_rate = 297000000
> > > > > > [    1.920602] reg = 0x90c00000
> > > > > > [    1.920605] _nkm.n = 3, nkm->n.offset = 0x1, nkm->n.shift = 8
> > > > > > [    1.920609] _nkm.k = 2, nkm->k.offset = 0x1, nkm->k.shift = 4
> > > > > > [    1.920612] _nkm.m = 10, nkm->m.offset = 0x1, nkm->m.shift = 0
> > > > > > [    1.920958] sun4i_dclk_set_rate div 6
> > > > > > [    1.920966] sun4i_dclk_recalc_rate: val = 6, rate = 29700000
> > > > > >
> > > > > > and clk_summary:
> > > > > >
> > > > > >     pll-video0                        1        1        1   297000000
> > > > > >         0     0  50000
> > > > > >        hdmi                           0        0        0   297000000
> > > > > >         0     0  50000
> > > > > >        tcon1                          0        0        0   297000000
> > > > > >         0     0  50000
> > > > > >        pll-mipi                       1        1        1   178200000
> > > > > >         0     0  50000
> > > > > >           tcon0                       2        2        1   178200000
> > > > > >         0     0  50000
> > > > > >              tcon-pixel-clock         1        1        1    29700000
> > > > > >         0     0  50000
> > > > > >        pll-video0-2x                  0        0        0   594000000
> > > > > >         0     0  50000
> > > > >
> > > > > This discussion is going nowhere. I'm telling you that your patch
> > > > > doesn't apply the divider you want on the proper clock, and you're
> > > > > replying that indeed, you're applying it on the wrong clock.
> > > > >
> > > > > It might work by accident in your case, but the board I have here
> > > > > clearly indicates otherwise, so there's two possible way out here:
> > > > >
> > > > >   - Either you apply that divider to the TCON *module* clock, and not
> > > > >     the dclk
> > > > >
> > > > >   - Or you point to somewhere in the allwinner code where the bpp /
> > > > >     lanes divider is used for the dclk divider.
> > > >
> > > > I don't know how to proceed further on this, as you say it might work
> > > > in accident but I have tested this in A33, A64 and R40 with 4
> > > > different DSI panels and one DSI-RGB bridge. All of them do use
> > > > PLL_MIPI (pll_rate) and it indeed depends on bpp/lanes
> > > >
> > > > 4-lane, 24-bit: Novatek NT35596 panel
> > > > 4-lane, 24-bit: Feiyang, FY07024di26a30d panel
> > > > 4-lane, 24-bit: Bananapi-s070wv20 panel
> > > > 2-lane, 24-bit: Techstar,ts8550b panel
> > > >
> > > > and
> > > >
> > > > 4-lane, 24-bit, ICN6211 DSI-to-RGB bridge panel
> > > >
> > > > All above listed panels and bridges are working as per BSP and do
> > > > follow bpp/lanes and for DIVIDER 4 no panel is working.
> > >
> > > Look. I'm not saying that there's no issue, I'm saying that your
> > > patch, applied to the clock you're applying it to, doesn't make sense
> > > and isn't what the BSP does.
> >
> > tcon-pixel clock is the rate that you want to achive on display side
> > and if you have 4 lanes 32bit or lanes and different bit number that
> > you need to have a clock that is able to put outside bits and speed
> > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > the display.
>
> So this is what the issue is then?
>
> This one does make sense, and you should just change the rate in the
> call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
>
> I'm still wondering why that hasn't been brought up in either the
> discussion or the commit log before though.
>
Something like this?

drivers/gpu/drm/sun4i/sun4i_tcon.c     | 20 +++++++++++---------
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 64c43ee6bd92..42560d5c327c 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
drm_display_mode *mode,
 }

 static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
-                                       const struct drm_display_mode *mode)
+                                       const struct drm_display_mode *mode,
+                                       u32 tcon_mul)
 {
        /* Configure the dot clock */
-       clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
+       clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);

        /* Set the resolution */
        regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
@@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
sun4i_tcon *tcon,
        u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
        u8 lanes = device->lanes;
        u32 block_space, start_delay;
-       u32 tcon_div;
+       u32 tcon_div, tcon_mul;

-       tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
-       tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
+       tcon->dclk_min_div = 4;
+       tcon->dclk_max_div = 127;

-       sun4i_tcon0_mode_set_common(tcon, mode);
+       tcon_mul = bpp / lanes;
+       sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);

        /* Set dithering if needed */
        sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
@@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
sun4i_tcon *tcon,
         */
        regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, &tcon_div);
        tcon_div &= GENMASK(6, 0);
-       block_space = mode->htotal * bpp / (tcon_div * lanes);
+       block_space = mode->htotal * tcon_div * tcon_mul;
        block_space -= mode->hdisplay + 40;

        regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
@@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
sun4i_tcon *tcon,

        tcon->dclk_min_div = 7;
        tcon->dclk_max_div = 7;
-       sun4i_tcon0_mode_set_common(tcon, mode);
+       sun4i_tcon0_mode_set_common(tcon, mode, 1);

        /* Set dithering if needed */
        sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
@@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
sun4i_tcon *tcon,

        tcon->dclk_min_div = 6;
        tcon->dclk_max_div = 127;
-       sun4i_tcon0_mode_set_common(tcon, mode);
+       sun4i_tcon0_mode_set_common(tcon, mode, 1);

        /* Set dithering if needed */
        sun4i_tcon0_mode_set_dithering(tcon, connector);
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index 5c3ad5be0690..a07090579f84 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -13,8 +13,6 @@
 #include <drm/drm_encoder.h>
 #include <drm/drm_mipi_dsi.h>

-#define SUN6I_DSI_TCON_DIV     4
-
 struct sun6i_dsi {
        struct drm_connector    connector;
        struct drm_encoder      encoder;


> > Now in burst mode I don't know how should work the calculation of
> > the clock for the require bandwidth and even I understand your
> > comment I would like to have your clock tree after you boot on the
> > display side and if it is possible I want to assemble a kit like you
> > have.
>
> The setup is probably going to be a bit difficult to reproduce, it's a
> prototype that I have that can't really be found anywhere. Jagan asked
> me on IRC for the reference, and he found the part, but it's unclear
> to me if it can be easily adapted to a common board.
>
> However, I'm not even sure we need this. I'll test the next version
> and let you know if it works.
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

^ permalink raw reply related

* Re: [PATCH RFC 2/4] OPP: Add and export helper to set bandwidth
From: Bjorn Andersson @ 2019-07-11 17:40 UTC (permalink / raw)
  To: Sibi Sankar
  Cc: viresh.kumar, nm, sboyd, georgi.djakov, agross, david.brown,
	robh+dt, mark.rutland, rjw, linux-arm-msm, devicetree,
	linux-kernel, linux-pm, saravanak
In-Reply-To: <20190627133424.4980-3-sibis@codeaurora.org>

On Thu 27 Jun 06:34 PDT 2019, Sibi Sankar wrote:

> Add and export 'dev_pm_opp_set_bw' to set the bandwidth
> levels associated with an OPP for a given frequency.
> 

While this looks quite reasonable I'm uncertain about the overall OPP
API.

With the profiling based (bwmon/llcc) approach we would acquire the peak
bandwidth from the OPP table and calculate the average dynamically,
based on measurements and heuristics.

For that I think we will have a struct dev_pm_opp at hand (e.g. from
devfreq_recommended_opp() or similar), from which we want to read the
peak value and then apply the icc vote. Or would we want to update the
avg bw and then apply the opp using a method like this? (In which case
we probably don't want to pass a freq, but a struct dev_pm_opp *, to
avoid the additional lookup)

Regards,
Bjorn

> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  drivers/opp/core.c     | 46 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pm_opp.h |  6 ++++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index c85c04dc2c7de..78f42960860d1 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -746,6 +746,52 @@ static int _set_required_opps(struct device *dev,
>  	return ret;
>  }
>  
> +/**
> + * dev_pm_opp_set_bw() - Configures OPP bandwidth levels
> + * @dev:	device for which we do this operation
> + * @freq:	bandwidth values to set with matching 'freq'
> + *
> + * This configures the bandwidth to the levels specified
> + * by the OPP corresponding to the given frequency.
> + *
> + * Return: 0 on success or a negative error value.
> + */
> +int dev_pm_opp_set_bw(struct device *dev, unsigned long freq)
> +{
> +	struct opp_table *opp_table;
> +	struct dev_pm_opp *opp;
> +	int ret = 0;
> +	int i;
> +
> +	opp = dev_pm_opp_find_freq_exact(dev, freq, true);
> +	if (IS_ERR(opp))
> +		return PTR_ERR(opp);
> +
> +	opp_table = _find_opp_table(dev);
> +	if (IS_ERR(opp_table)) {
> +		dev_err(dev, "%s: device opp table doesn't exist\n", __func__);
> +		ret = PTR_ERR(opp_table);
> +		goto put_opp;
> +	}
> +
> +	if (IS_ERR_OR_NULL(opp_table->paths)) {
> +		ret = -ENODEV;
> +		goto put_opp_table;
> +	}
> +
> +	for (i = 0; i < opp_table->path_count; i++) {
> +		ret = icc_set_bw(opp_table->paths[i], opp->bandwidth[i].avg,
> +				 opp->bandwidth[i].peak);
> +	}
> +
> +put_opp_table:
> +	dev_pm_opp_put_opp_table(opp_table);
> +put_opp:
> +	dev_pm_opp_put(opp);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_opp_set_bw);
> +
>  /**
>   * dev_pm_opp_set_rate() - Configure new OPP based on frequency
>   * @dev:	 device for which we do this operation
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index a17c462974851..1cdc2d0a2b20e 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -152,6 +152,7 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names
>  void dev_pm_opp_detach_genpd(struct opp_table *opp_table);
>  int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate);
>  int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
> +int dev_pm_opp_set_bw(struct device *dev, unsigned long freq);
>  int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
>  int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
>  void dev_pm_opp_remove_table(struct device *dev);
> @@ -336,6 +337,11 @@ static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_f
>  	return -ENOTSUPP;
>  }
>  
> +static inline int dev_pm_opp_set_bw(struct device *dev, unsigned long freq)
> +{
> +	return -ENOTSUPP;
> +}
> +
>  static inline int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask)
>  {
>  	return -ENOTSUPP;
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

^ permalink raw reply

* Re: [PATCH 1/3] dt-bindings: dma: Add YAML schemas for the generic DMA bindings
From: Rob Herring @ 2019-07-11 17:33 UTC (permalink / raw)
  To: Maxime Ripard, Jon Hunter, Peter Ujfalusi
  Cc: Mark Rutland, devicetree, Chen-Yu Tsai, Vinod Koul,
	open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM, Frank Rowand,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20190711092158.14678-1-maxime.ripard@bootlin.com>

On Thu, Jul 11, 2019 at 3:34 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> The DMA controllers and consumers have a bunch of generic properties that
> are needed in a device tree. Add a YAML schemas for those.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  .../devicetree/bindings/dma/dma-consumer.yaml |  60 +++++++++

This already exists in the dt-schema/schemas/dma/dma.yaml though not
the descriptions because that needs relicensing.

Looks like we need NVidia's (Jon H) and TI's (Peter U) permission.

>  .../bindings/dma/dma-controller.yaml          |  79 ++++++++++++
>  Documentation/devicetree/bindings/dma/dma.txt | 114 +-----------------
>  3 files changed, 140 insertions(+), 113 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/dma/dma-consumer.yaml
>  create mode 100644 Documentation/devicetree/bindings/dma/dma-controller.yaml
>
> diff --git a/Documentation/devicetree/bindings/dma/dma-consumer.yaml b/Documentation/devicetree/bindings/dma/dma-consumer.yaml
> new file mode 100644
> index 000000000000..2f6315863ad1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/dma-consumer.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/dma-consumer.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: DMA Consumer Generic Binding
> +
> +maintainers:
> +  - Vinod Koul <vkoul@kernel.org>
> +
> +select: true
> +
> +properties:
> +  dmas:
> +    description:
> +      List of one or more DMA specifiers, each consisting of
> +          - A phandle pointing to DMA controller node
> +          - A number of integer cells, as determined by the
> +            \#dma-cells property in the node referenced by phandle
> +            containing DMA controller specific information. This
> +            typically contains a DMA request line number or a
> +            channel number, but can contain any data that is
> +            required for configuring a channel.
> +
> +  dma-names:
> +    description:
> +      Contains one identifier string for each DMA specifier in the
> +      dmas property. The specific strings that can be used are defined
> +      in the binding of the DMA client device.  Multiple DMA
> +      specifiers can be used to represent alternatives and in this
> +      case the dma-names for those DMA specifiers must be identical
> +      (see examples).
> +
> +dependencies:
> +  dma-names: [ dmas ]
> +
> +examples:
> +  - |
> +    /* A device with one DMA read channel, one DMA write channel */
> +    i2c1: i2c@1 {
> +         /* ... */
> +         dmas = <&dma 2>,      /* read channel */
> +                <&dma 3>;      /* write channel */
> +        dma-names = "rx", "tx";
> +        /* ... */
> +    };
> +
> +  - |
> +    /* A single read-write channel with three alternative DMA controllers */
> +    dmas = <&dma1 5>, <&dma2 7>, <&dma3 2>;
> +    dma-names = "rx-tx", "rx-tx", "rx-tx";
> +
> +  - |
> +    /* A device with three channels, one of which has two alternatives */
> +    dmas = <&dma1 2>,          /* read channel */
> +           <&dma1 3>,          /* write channel */
> +           <&dma2 0>,          /* error read */
> +           <&dma3 0>;          /* alternative error read */
> +    dma-names = "rx", "tx", "error", "error";
> diff --git a/Documentation/devicetree/bindings/dma/dma-controller.yaml b/Documentation/devicetree/bindings/dma/dma-controller.yaml
> new file mode 100644
> index 000000000000..17c650131b78
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/dma-controller.yaml
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/dma-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: DMA Controller Generic Binding
> +
> +maintainers:
> +  - Vinod Koul <vkoul@kernel.org>
> +
> +description:
> +  Generic binding to provide a way for a driver using DMA Engine to
> +  retrieve the DMA request or channel information that goes from a
> +  hardware device to a DMA controller.
> +
> +properties:
> +  $nodename:
> +    pattern: "^dma-controller(@.*)?$"
> +
> +  "#dma-cells":
> +    # minimum: 1
> +    description:
> +      Used to provide DMA controller specific information.
> +
> +  dma-channel-masks:
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    description:
> +      Bitmask of available DMA channels in ascending order that are
> +      not reserved by firmware and are available to the
> +      kernel. i.e. first channel corresponds to LSB.
> +
> +  dma-channels:
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    description:
> +      Number of DMA channels supported by the controller.
> +
> +  dma-masters:
> +    $ref: /schemas/types.yaml#definitions/phandle-array
> +    description:
> +      DMA routers are transparent IP blocks used to route DMA request
> +      lines from devices to the DMA controller. Some SoCs (like TI
> +      DRA7x) have more peripherals integrated with DMA requests than
> +      what the DMA controller can handle directly.
> +
> +      In such a case, dma-masters is an array of phandle to the DMA
> +      controllers the router can direct the signal to.
> +
> +  dma-requests:
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    description:
> +      Number of DMA request signals supported by the controller.
> +
> +examples:
> +  - |
> +    dma: dma@48000000 {

dma-controller@...

This is a case where I'd like some check that the schema is actually
applied to the schema's example.

> +        compatible = "ti,omap-sdma";
> +        reg = <0x48000000 0x1000>;
> +        interrupts = <0 12 0x4
> +                      0 13 0x4
> +                      0 14 0x4
> +                      0 15 0x4>;
> +        #dma-cells = <1>;
> +        dma-channels = <32>;
> +        dma-requests = <127>;
> +        dma-channel-mask = <0xfffe>;
> +    };
> +
> +  - |
> +    sdma_xbar: dma-router@4a002b78 {
> +        compatible = "ti,dra7-dma-crossbar";
> +        reg = <0x4a002b78 0xfc>;
> +        #dma-cells = <1>;
> +        dma-requests = <205>;
> +        ti,dma-safe-map = <0>;
> +        dma-masters = <&sdma>;
> +    };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/dma/dma.txt b/Documentation/devicetree/bindings/dma/dma.txt
> index eeb4e4d1771e..90a67a016a48 100644
> --- a/Documentation/devicetree/bindings/dma/dma.txt
> +++ b/Documentation/devicetree/bindings/dma/dma.txt
> @@ -1,113 +1 @@
> -* Generic DMA Controller and DMA request bindings
> -
> -Generic binding to provide a way for a driver using DMA Engine to retrieve the
> -DMA request or channel information that goes from a hardware device to a DMA
> -controller.
> -
> -
> -* DMA controller
> -
> -Required property:
> -- #dma-cells:          Must be at least 1. Used to provide DMA controller
> -                       specific information. See DMA client binding below for
> -                       more details.
> -
> -Optional properties:
> -- dma-channels:        Number of DMA channels supported by the controller.
> -- dma-requests:        Number of DMA request signals supported by the
> -                       controller.
> -- dma-channel-mask:    Bitmask of available DMA channels in ascending order
> -                       that are not reserved by firmware and are available to
> -                       the kernel. i.e. first channel corresponds to LSB.
> -
> -Example:
> -
> -       dma: dma@48000000 {
> -               compatible = "ti,omap-sdma";
> -               reg = <0x48000000 0x1000>;
> -               interrupts = <0 12 0x4
> -                             0 13 0x4
> -                             0 14 0x4
> -                             0 15 0x4>;
> -               #dma-cells = <1>;
> -               dma-channels = <32>;
> -               dma-requests = <127>;
> -               dma-channel-mask = <0xfffe>
> -       };
> -
> -* DMA router
> -
> -DMA routers are transparent IP blocks used to route DMA request lines from
> -devices to the DMA controller. Some SoCs (like TI DRA7x) have more peripherals
> -integrated with DMA requests than what the DMA controller can handle directly.
> -
> -Required property:
> -- dma-masters:         phandle of the DMA controller or list of phandles for
> -                       the DMA controllers the router can direct the signal to.
> -- #dma-cells:          Must be at least 1. Used to provide DMA router specific
> -                       information. See DMA client binding below for more
> -                       details.
> -
> -Optional properties:
> -- dma-requests:        Number of incoming request lines the router can handle.
> -- In the node pointed by the dma-masters:
> -       - dma-requests: The router driver might need to look for this in order
> -                       to configure the routing.
> -
> -Example:
> -       sdma_xbar: dma-router@4a002b78 {
> -               compatible = "ti,dra7-dma-crossbar";
> -               reg = <0x4a002b78 0xfc>;
> -               #dma-cells = <1>;
> -               dma-requests = <205>;
> -               ti,dma-safe-map = <0>;
> -               dma-masters = <&sdma>;
> -       };
> -
> -* DMA client
> -
> -Client drivers should specify the DMA property using a phandle to the controller
> -followed by DMA controller specific data.
> -
> -Required property:
> -- dmas:                        List of one or more DMA specifiers, each consisting of
> -                       - A phandle pointing to DMA controller node
> -                       - A number of integer cells, as determined by the
> -                         #dma-cells property in the node referenced by phandle
> -                         containing DMA controller specific information. This
> -                         typically contains a DMA request line number or a
> -                         channel number, but can contain any data that is
> -                         required for configuring a channel.
> -- dma-names:           Contains one identifier string for each DMA specifier in
> -                       the dmas property. The specific strings that can be used
> -                       are defined in the binding of the DMA client device.
> -                       Multiple DMA specifiers can be used to represent
> -                       alternatives and in this case the dma-names for those
> -                       DMA specifiers must be identical (see examples).
> -
> -Examples:
> -
> -1. A device with one DMA read channel, one DMA write channel:
> -
> -       i2c1: i2c@1 {
> -               ...
> -               dmas = <&dma 2          /* read channel */
> -                       &dma 3>;        /* write channel */
> -               dma-names = "rx", "tx";
> -               ...
> -       };
> -
> -2. A single read-write channel with three alternative DMA controllers:
> -
> -       dmas = <&dma1 5
> -               &dma2 7
> -               &dma3 2>;
> -       dma-names = "rx-tx", "rx-tx", "rx-tx";
> -
> -3. A device with three channels, one of which has two alternatives:
> -
> -       dmas = <&dma1 2                 /* read channel */
> -               &dma1 3                 /* write channel */
> -               &dma2 0                 /* error read */
> -               &dma3 0>;               /* alternative error read */
> -       dma-names = "rx", "tx", "error", "error";
> +This file has been moved to dma-controller.yaml.
> --
> 2.21.0
>

^ permalink raw reply

* Re: [PATCH 1/6] dt-bindings: pinctrl: aspeed: Document AST2600 pinmux
From: Rob Herring @ 2019-07-11 17:18 UTC (permalink / raw)
  To: Andrew Jeffery
  Cc: open list:GPIO SUBSYSTEM, Linus Walleij, Mark Rutland,
	Joel Stanley, Ryan Chen, Johnny Huang, linux-aspeed, devicetree,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel@vger.kernel.org
In-Reply-To: <20190711041942.23202-2-andrew@aj.id.au>

On Wed, Jul 10, 2019 at 10:19 PM Andrew Jeffery <andrew@aj.id.au> wrote:
>
> The AST260 differs from the 2400 and 2500 in that it supports multiple
> groups for a subset of functions.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>  .../pinctrl/aspeed,ast2600-pinctrl.yaml       | 128 ++++++++++++++++++
>  1 file changed, 128 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml
> new file mode 100644
> index 000000000000..dd31f8e62433
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml
> @@ -0,0 +1,128 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/aspeed,ast2600-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ASPEED AST2600 Pin Controller
> +
> +maintainers:
> +  - Andrew Jeffery <andrew@aj.id.au>
> +
> +description: |+
> +  The pin controller node should be the child of a syscon node with the
> +  required property:
> +
> +  - compatible: Should be one of the following:
> +                "aspeed,ast2600-scu", "syscon", "simple-mfd"
> +
> +  Refer to the the bindings described in
> +  Documentation/devicetree/bindings/mfd/syscon.txt
> +
> +properties:
> +  compatible:
> +    const: aspeed,ast2600-pinctrl
> +
> +patternProperties:
> +  '^.*$':
> +    if:
> +      type: object
> +    then:
> +      patternProperties:
> +        "^function$":

That's a fixed string, not a pattern, so just put under 'properties'.

> +          allOf:
> +            - $ref: "/schemas/types.yaml#/definitions/string"
> +            - enum: [ "ADC0", "ADC1", "ADC10", "ADC11", "ADC12", "ADC13",

You can drop the quoting here. I wouldn't really care, but yours is
one of the few right now and everyone will copy.

> +              "ADC14", "ADC15", "ADC2", "ADC3", "ADC4", "ADC5", "ADC6", "ADC7",
> +              "ADC8", "ADC9", "BMCINT", "ESPI", "ESPIALT", "FSI1", "FSI2",
> +              "FWSPIABR", "FWSPID", "FWSPIWP", "GPIT0", "GPIT1", "GPIT2",
> +              "GPIT3", "GPIT4", "GPIT5", "GPIT6", "GPIT7", "GPIU0", "GPIU1",
> +              "GPIU2", "GPIU3", "GPIU4", "GPIU5", "GPIU6", "GPIU7", "I2C1",
> +              "I2C10", "I2C11", "I2C12", "I2C13", "I2C14", "I2C15", "I2C16",
> +              "I2C2", "I2C3", "I2C4", "I2C5", "I2C6", "I2C7", "I2C8", "I2C9",
> +              "I3C3", "I3C4", "I3C5", "I3C6", "JTAGM", "LHPD", "LHSIRQ", "LPC",
> +              "LPCHC", "LPCPD", "LPCPME", "LPCSMI", "LSIRQ", "MACLINK1",
> +              "MACLINK2", "MACLINK3", "MACLINK4", "MDIO1", "MDIO2", "MDIO3",
> +              "MDIO4", "NCTS1", "NCTS2", "NCTS3", "NCTS4", "NDCD1", "NDCD2",
> +              "NDCD3", "NDCD4", "NDSR1", "NDSR2", "NDSR3", "NDSR4", "NDTR1",
> +              "NDTR2", "NDTR3", "NDTR4", "NRI1", "NRI2", "NRI3", "NRI4",
> +              "NRTS1", "NRTS2", "NRTS3", "NRTS4", "OSCCLK", "PEWAKE", "PWM0",
> +              "PWM1", "PWM10", "PWM11", "PWM12", "PWM13", "PWM14", "PWM15",
> +              "PWM2", "PWM3", "PWM4", "PWM5", "PWM6", "PWM7", "PWM8", "PWM9",
> +              "RGMII1", "RGMII2", "RGMII3", "RGMII4", "RMII1", "RMII2",
> +              "RMII3", "RMII4", "RXD1", "RXD2", "RXD3", "RXD4", "SALT1",
> +              "SALT10", "SALT11", "SALT12", "SALT13", "SALT14", "SALT15",
> +              "SALT16", "SALT2", "SALT3", "SALT4", "SALT5", "SALT6", "SALT7",
> +              "SALT8", "SALT9", "SD1", "SD2", "SD3", "SD3DAT4", "SD3DAT5",
> +              "SD3DAT6", "SD3DAT7", "SGPM1", "SGPS1", "SIOONCTRL", "SIOPBI",
> +              "SIOPBO", "SIOPWREQ", "SIOPWRGD", "SIOS3", "SIOS5", "SIOSCI",
> +              "SPI1", "SPI1ABR", "SPI1CS1", "SPI1WP", "SPI2", "SPI2CS1",
> +              "SPI2CS2", "TACH0", "TACH1", "TACH10", "TACH11", "TACH12",
> +              "TACH13", "TACH14", "TACH15", "TACH2", "TACH3", "TACH4", "TACH5",
> +              "TACH6", "TACH7", "TACH8", "TACH9", "THRU0", "THRU1", "THRU2",
> +              "THRU3", "TXD1", "TXD2", "TXD3", "TXD4", "UART10", "UART11",
> +              "UART12", "UART13", "UART6", "UART7", "UART8", "UART9", "VB",
> +              "VGAHS", "VGAVS", "WDTRST1", "WDTRST2", "WDTRST3", "WDTRST4", ]
> +        "^groups$":
> +          allOf:
> +            - $ref: "/schemas/types.yaml#/definitions/string"
> +            - enum: [ "ADC0", "ADC1", "ADC10", "ADC11", "ADC12", "ADC13",
> +              "ADC14", "ADC15", "ADC2", "ADC3", "ADC4", "ADC5", "ADC6", "ADC7",
> +              "ADC8", "ADC9", "BMCINT", "ESPI", "ESPIALT", "FSI1", "FSI2",
> +              "FWSPIABR", "FWSPID", "FWQSPID", "FWSPIWP", "GPIT0", "GPIT1",
> +              "GPIT2", "GPIT3", "GPIT4", "GPIT5", "GPIT6", "GPIT7", "GPIU0",
> +              "GPIU1", "GPIU2", "GPIU3", "GPIU4", "GPIU5", "GPIU6", "GPIU7",
> +              "HVI3C3", "HVI3C4", "I2C1", "I2C10", "I2C11", "I2C12", "I2C13",
> +              "I2C14", "I2C15", "I2C16", "I2C2", "I2C3", "I2C4", "I2C5",
> +              "I2C6", "I2C7", "I2C8", "I2C9", "I3C3", "I3C4", "I3C5", "I3C6",
> +              "JTAGM", "LHPD", "LHSIRQ", "LPC", "LPCHC", "LPCPD", "LPCPME",
> +              "LPCSMI", "LSIRQ", "MACLINK1", "MACLINK2", "MACLINK3",
> +              "MACLINK4", "MDIO1", "MDIO2", "MDIO3", "MDIO4", "NCTS1", "NCTS2",
> +              "NCTS3", "NCTS4", "NDCD1", "NDCD2", "NDCD3", "NDCD4", "NDSR1",
> +              "NDSR2", "NDSR3", "NDSR4", "NDTR1", "NDTR2", "NDTR3", "NDTR4",
> +              "NRI1", "NRI2", "NRI3", "NRI4", "NRTS1", "NRTS2", "NRTS3",
> +              "NRTS4", "OSCCLK", "PEWAKE", "PWM0", "PWM1", "PWM10G0",
> +              "PWM10G1", "PWM11G0", "PWM11G1", "PWM12G0", "PWM12G1", "PWM13G0",
> +              "PWM13G1", "PWM14G0", "PWM14G1", "PWM15G0", "PWM15G1", "PWM2",
> +              "PWM3", "PWM4", "PWM5", "PWM6", "PWM7", "PWM8G0", "PWM8G1",
> +              "PWM9G0", "PWM9G1", "QSPI1", "QSPI2", "RGMII1", "RGMII2",
> +              "RGMII3", "RGMII4", "RMII1", "RMII2", "RMII3", "RMII4", "RXD1",
> +              "RXD2", "RXD3", "RXD4", "SALT1", "SALT10G0", "SALT10G1",
> +              "SALT11G0", "SALT11G1", "SALT12G0", "SALT12G1", "SALT13G0",
> +              "SALT13G1", "SALT14G0", "SALT14G1", "SALT15G0", "SALT15G1",
> +              "SALT16G0", "SALT16G1", "SALT2", "SALT3", "SALT4", "SALT5",
> +              "SALT6", "SALT7", "SALT8", "SALT9G0", "SALT9G1", "SD1", "SD2",
> +              "SD3", "SD3DAT4", "SD3DAT5", "SD3DAT6", "SD3DAT7", "SGPM1",
> +              "SGPS1", "SIOONCTRL", "SIOPBI", "SIOPBO", "SIOPWREQ", "SIOPWRGD",
> +              "SIOS3", "SIOS5", "SIOSCI", "SPI1", "SPI1ABR", "SPI1CS1",
> +              "SPI1WP", "SPI2", "SPI2CS1", "SPI2CS2", "TACH0", "TACH1",
> +              "TACH10", "TACH11", "TACH12", "TACH13", "TACH14", "TACH15",
> +              "TACH2", "TACH3", "TACH4", "TACH5", "TACH6", "TACH7", "TACH8",
> +              "TACH9", "THRU0", "THRU1", "THRU2", "THRU3", "TXD1", "TXD2",
> +              "TXD3", "TXD4", "UART10", "UART11", "UART12G0", "UART12G1",
> +              "UART13G0", "UART13G1", "UART6", "UART7", "UART8", "UART9", "VB",
> +              "VGAHS", "VGAVS", "WDTRST1", "WDTRST2", "WDTRST3", "WDTRST4", ]
> +
> +required:
> +  - compatible
> +
> +examples:
> +  - |
> +    syscon: scu@1e6e2000 {
> +        compatible = "aspeed,ast2600-scu", "syscon", "simple-mfd";
> +        reg = <0x1e6e2000 0xf6c>;
> +
> +        pinctrl: pinctrl {
> +            compatible = "aspeed,g6-pinctrl";
> +
> +            pinctrl_pwm10g1_default: pwm10g1_default {
> +                function = "PWM10";
> +                groups = "PWM10G1";
> +            };
> +
> +            pinctrl_gpioh0_unbiased_default: gpioh0 {
> +                pins = "A18";
> +                bias-disable;

This node and properties aren't getting checked. Need to figure out
how to do that. That may mean needing some structure to the node
names. You can worry about that later I suppose.

Rob

^ permalink raw reply

* Re: [PATCH 2/6] irqchip/irq-pruss-intc: Add a PRUSS irqchip driver for PRUSS interrupts
From: David Lechner @ 2019-07-11 16:45 UTC (permalink / raw)
  To: Suman Anna, Marc Zyngier, Rob Herring, Thomas Gleixner,
	Jason Cooper
  Cc: Tony Lindgren, Andrew F. Davis, Roger Quadros, Lokesh Vutla,
	Grygorii Strashko, Sekhar Nori, Murali Karicheri, devicetree,
	linux-omap, linux-arm-kernel, linux-kernel
In-Reply-To: <20190708035243.12170-3-s-anna@ti.com>

On 7/7/19 10:52 PM, Suman Anna wrote:
> From: "Andrew F. Davis" <afd@ti.com>
> 
> The Programmable Real-Time Unit Subsystem (PRUSS) contains a local
> interrupt controller (INTC) that can handle various system input events
> and post interrupts back to the device-level initiators. The INTC can
> support upto 64 input events with individual control configuration and
> hardware prioritization. These events are mapped onto 10 output interrupt
> lines through two levels of many-to-one mapping support. Different
> interrupt lines are routed to the individual PRU cores or to the host
> CPU, or to other devices on the SoC. Some of these events are sourced
> from peripherals or other sub-modules within that PRUSS, while a few
> others are sourced from SoC-level peripherals/devices.
> 
> The PRUSS INTC platform driver manages this PRUSS interrupt controller
> and implements an irqchip driver to provide a Linux standard way for
> the PRU client users to enable/disable/ack/re-trigger a PRUSS system
> event. The system events to interrupt channels and host interrupts
> relies on the mapping configuration provided either through the PRU
> firmware blob or via the PRU application's device tree node. The
> mappings will be programmed during the boot/shutdown of a PRU core.
> 
> The PRUSS INTC module is reference counted during the interrupt
> setup phase through the irqchip's irq_request_resources() and
> irq_release_resources() ops. This restricts the module from being
> removed as long as there are active interrupt users.
> 
> The driver currently supports and can be built for OMAP architecture
> based AM335x, AM437x and AM57xx SoCs; Keystone2 architecture based
> 66AK2G SoCs and Davinci architecture based OMAP-L13x/AM18x/DA850 SoCs.
> All of these SoCs support 64 system events, 10 interrupt channels and
> 10 output interrupt lines per PRUSS INTC with a few SoC integration
> differences.
> 
> NOTE:
> Each PRU-ICSS's INTC on AM57xx SoCs is preceded by a Crossbar that
> enables multiple external events to be routed to a specific number
> of input interrupt events. Any non-default external interrupt event
> directed towards PRUSS needs this crossbar to be setup properly.

The all of the explanations above are very helpful. Great work.

> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> Prior version: https://patchwork.kernel.org/patch/10795761/
> 
>   drivers/irqchip/Kconfig          |  10 +
>   drivers/irqchip/Makefile         |   1 +
>   drivers/irqchip/irq-pruss-intc.c | 352 +++++++++++++++++++++++++++++++
>   3 files changed, 363 insertions(+)
>   create mode 100644 drivers/irqchip/irq-pruss-intc.c
> 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 659c5e0fb835..b0a9479d527c 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -447,6 +447,16 @@ config TI_SCI_INTA_IRQCHIP
>   	  If you wish to use interrupt aggregator irq resources managed by the
>   	  TI System Controller, say Y here. Otherwise, say N.
>   
> +config TI_PRUSS_INTC
> +	tristate "TI PRU-ICSS Interrupt Controller"
> +	depends on ARCH_DAVINCI || SOC_AM33XX || SOC_AM437X || SOC_DRA7XX || ARCH_KEYSTONE
> +	select IRQ_DOMAIN
> +	help
> +	   This enables support for the PRU-ICSS Local Interrupt Controller
> +	   present within a PRU-ICSS subsystem present on various TI SoCs.
> +	   The PRUSS INTC enables various interrupts to be routed to multiple
> +	   different processors within the SoC.
> +
>   endmenu
>   
>   config SIFIVE_PLIC
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 606a003a0000..717f1d49e549 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -100,3 +100,4 @@ obj-$(CONFIG_MADERA_IRQ)		+= irq-madera.o
>   obj-$(CONFIG_LS1X_IRQ)			+= irq-ls1x.o
>   obj-$(CONFIG_TI_SCI_INTR_IRQCHIP)	+= irq-ti-sci-intr.o
>   obj-$(CONFIG_TI_SCI_INTA_IRQCHIP)	+= irq-ti-sci-inta.o
> +obj-$(CONFIG_TI_PRUSS_INTC)		+= irq-pruss-intc.o
> diff --git a/drivers/irqchip/irq-pruss-intc.c b/drivers/irqchip/irq-pruss-intc.c
> new file mode 100644
> index 000000000000..d62186ad1be4
> --- /dev/null
> +++ b/drivers/irqchip/irq-pruss-intc.c
> @@ -0,0 +1,352 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PRU-ICSS INTC IRQChip driver for various TI SoCs
> + *
> + * Copyright (C) 2016-2019 Texas Instruments Incorporated - http://www.ti.com/
> + *	Andrew F. Davis <afd@ti.com>
> + *	Suman Anna <s-anna@ti.com>
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +
> +/*
> + * Number of host interrupts reaching the main MPU sub-system. Note that this
> + * is not the same as the total number of host interrupts supported by the PRUSS
> + * INTC instance
> + */
> +#define MAX_NUM_HOST_IRQS	8
> +
> +/* minimum starting host interrupt number for MPU */
> +#define MIN_PRU_HOST_INT	2
> +
> +/* maximum number of system events */
> +#define MAX_PRU_SYS_EVENTS	64
> +
> +/* PRU_ICSS_INTC registers */
> +#define PRU_INTC_REVID		0x0000
> +#define PRU_INTC_CR		0x0004
> +#define PRU_INTC_GER		0x0010
> +#define PRU_INTC_GNLR		0x001c
> +#define PRU_INTC_SISR		0x0020
> +#define PRU_INTC_SICR		0x0024
> +#define PRU_INTC_EISR		0x0028
> +#define PRU_INTC_EICR		0x002c
> +#define PRU_INTC_HIEISR		0x0034
> +#define PRU_INTC_HIDISR		0x0038
> +#define PRU_INTC_GPIR		0x0080
> +#define PRU_INTC_SRSR0		0x0200
> +#define PRU_INTC_SRSR1		0x0204
> +#define PRU_INTC_SECR0		0x0280
> +#define PRU_INTC_SECR1		0x0284
> +#define PRU_INTC_ESR0		0x0300
> +#define PRU_INTC_ESR1		0x0304
> +#define PRU_INTC_ECR0		0x0380
> +#define PRU_INTC_ECR1		0x0384
> +#define PRU_INTC_CMR(x)		(0x0400 + (x) * 4)
> +#define PRU_INTC_HMR(x)		(0x0800 + (x) * 4)
> +#define PRU_INTC_HIPIR(x)	(0x0900 + (x) * 4)
> +#define PRU_INTC_SIPR0		0x0d00
> +#define PRU_INTC_SIPR1		0x0d04
> +#define PRU_INTC_SITR0		0x0d80
> +#define PRU_INTC_SITR1		0x0d84
> +#define PRU_INTC_HINLR(x)	(0x1100 + (x) * 4)
> +#define PRU_INTC_HIER		0x1500
> +
> +/* HIPIR register bit-fields */
> +#define INTC_HIPIR_NONE_HINT	0x80000000

Unused macro. See below.

> +
> +/**
> + * struct pruss_intc - PRUSS interrupt controller structure
> + * @irqs: kernel irq numbers corresponding to PRUSS host interrupts
> + * @base: base virtual address of INTC register space
> + * @irqchip: irq chip for this interrupt controller
> + * @domain: irq domain for this interrupt controller
> + * @lock: mutex to serialize access to INTC
> + * @host_mask: indicate which HOST IRQs are enabled
> + */
> +struct pruss_intc {
> +	unsigned int irqs[MAX_NUM_HOST_IRQS];
> +	void __iomem *base;
> +	struct irq_chip *irqchip;
> +	struct irq_domain *domain;
> +	struct mutex lock; /* PRUSS INTC lock */
> +	u32 host_mask;
> +};
> +
> +static inline u32 pruss_intc_read_reg(struct pruss_intc *intc, unsigned int reg)
> +{
> +	return readl_relaxed(intc->base + reg);
> +}
> +
> +static inline void pruss_intc_write_reg(struct pruss_intc *intc,
> +					unsigned int reg, u32 val)
> +{
> +	writel_relaxed(val, intc->base + reg);
> +}
> +
> +static int pruss_intc_check_write(struct pruss_intc *intc, unsigned int reg,
> +				  unsigned int sysevent)
> +{
> +	if (!intc)
> +		return -EINVAL;
> +
> +	if (sysevent >= MAX_PRU_SYS_EVENTS)
> +		return -EINVAL;
> +
> +	pruss_intc_write_reg(intc, reg, sysevent);
> +
> +	return 0;
> +}
> +
> +static void pruss_intc_init(struct pruss_intc *intc)
> +{
> +	int i;
> +
> +	/* configure polarity to active high for all system interrupts */
> +	pruss_intc_write_reg(intc, PRU_INTC_SIPR0, 0xffffffff);
> +	pruss_intc_write_reg(intc, PRU_INTC_SIPR1, 0xffffffff);
> +
> +	/* configure type to pulse interrupt for all system interrupts */
> +	pruss_intc_write_reg(intc, PRU_INTC_SITR0, 0);
> +	pruss_intc_write_reg(intc, PRU_INTC_SITR1, 0);
> +
> +	/* clear all 16 interrupt channel map registers */
> +	for (i = 0; i < 16; i++)
> +		pruss_intc_write_reg(intc, PRU_INTC_CMR(i), 0);
> +
> +	/* clear all 3 host interrupt map registers */
> +	for (i = 0; i < 3; i++)
> +		pruss_intc_write_reg(intc, PRU_INTC_HMR(i), 0);
> +}
> +
> +static void pruss_intc_irq_ack(struct irq_data *data)
> +{
> +	struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
> +	unsigned int hwirq = data->hwirq;
> +
> +	pruss_intc_check_write(intc, PRU_INTC_SICR, hwirq);
> +}
> +
> +static void pruss_intc_irq_mask(struct irq_data *data)
> +{
> +	struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
> +	unsigned int hwirq = data->hwirq;
> +
> +	pruss_intc_check_write(intc, PRU_INTC_EICR, hwirq);
> +}
> +
> +static void pruss_intc_irq_unmask(struct irq_data *data)
> +{
> +	struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
> +	unsigned int hwirq = data->hwirq;
> +
> +	pruss_intc_check_write(intc, PRU_INTC_EISR, hwirq);
> +}
> +
> +static int pruss_intc_irq_retrigger(struct irq_data *data)
> +{
> +	struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
> +	unsigned int hwirq = data->hwirq;
> +
> +	return pruss_intc_check_write(intc, PRU_INTC_SISR, hwirq);
> +}
> +
> +static int pruss_intc_irq_reqres(struct irq_data *data)
> +{
> +	if (!try_module_get(THIS_MODULE))
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +static void pruss_intc_irq_relres(struct irq_data *data)
> +{
> +	module_put(THIS_MODULE);
> +}
> +
> +static int pruss_intc_irq_domain_map(struct irq_domain *d, unsigned int virq,
> +				     irq_hw_number_t hw)
> +{
> +	struct pruss_intc *intc = d->host_data;
> +
> +	irq_set_chip_data(virq, intc);
> +	irq_set_chip_and_handler(virq, intc->irqchip, handle_level_irq);
> +
> +	return 0;
> +}
> +
> +static void pruss_intc_irq_domain_unmap(struct irq_domain *d, unsigned int virq)
> +{
> +	irq_set_chip_and_handler(virq, NULL, NULL);
> +	irq_set_chip_data(virq, NULL);
> +}
> +
> +static const struct irq_domain_ops pruss_intc_irq_domain_ops = {
> +	.xlate	= irq_domain_xlate_onecell,
> +	.map	= pruss_intc_irq_domain_map,
> +	.unmap	= pruss_intc_irq_domain_unmap,
> +};
> +
> +static void pruss_intc_irq_handler(struct irq_desc *desc)
> +{
> +	unsigned int irq = irq_desc_get_irq(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct pruss_intc *intc = irq_get_handler_data(irq);
> +	u32 hipir;
> +	unsigned int virq;
> +	int i, hwirq;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	/* find our host irq number */
> +	for (i = 0; i < MAX_NUM_HOST_IRQS; i++)
> +		if (intc->irqs[i] == irq)
> +			break;
> +	if (i == MAX_NUM_HOST_IRQS)
> +		goto err;
> +
> +	i += MIN_PRU_HOST_INT;
> +
> +	/* get highest priority pending PRUSS system event */
> +	hipir = pruss_intc_read_reg(intc, PRU_INTC_HIPIR(i));
> +	while (!(hipir & BIT(31))) {

Is BIT(31) here supposed to be INTC_HIPIR_NONE_HINT?

> +		hwirq = hipir & GENMASK(9, 0);
> +		virq = irq_linear_revmap(intc->domain, hwirq);
> +
> +		/*
> +		 * NOTE: manually ACK any system events that do not have a
> +		 * handler mapped yet
> +		 */
> +		if (unlikely(!virq))
> +			pruss_intc_check_write(intc, PRU_INTC_SICR, hwirq);
> +		else
> +			generic_handle_irq(virq);
> +
> +		/* get next system event */
> +		hipir = pruss_intc_read_reg(intc, PRU_INTC_HIPIR(i));
> +	}
> +err:
> +	chained_irq_exit(chip, desc);
> +}
> +
> +static int pruss_intc_probe(struct platform_device *pdev)
> +{
> +	static const char * const irq_names[] = {
> +				"host0", "host1", "host2", "host3",
> +				"host4", "host5", "host6", "host7", };
> +	struct device *dev = &pdev->dev;
> +	struct pruss_intc *intc;
> +	struct resource *res;
> +	struct irq_chip *irqchip;
> +	int i, irq;
> +
> +	intc = devm_kzalloc(dev, sizeof(*intc), GFP_KERNEL);
> +	if (!intc)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, intc);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	intc->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(intc->base)) {
> +		dev_err(dev, "failed to parse and map intc memory resource\n");
> +		return PTR_ERR(intc->base);
> +	}
> +
> +	dev_dbg(dev, "intc memory: pa %pa size 0x%zx va %pK\n", &res->start,
> +		(size_t)resource_size(res), intc->base);
> +
> +	mutex_init(&intc->lock);
> +
> +	pruss_intc_init(intc);
> +
> +	irqchip = devm_kzalloc(dev, sizeof(*irqchip), GFP_KERNEL);
> +	if (!irqchip)
> +		return -ENOMEM;
> +
> +	irqchip->irq_ack = pruss_intc_irq_ack;
> +	irqchip->irq_mask = pruss_intc_irq_mask;
> +	irqchip->irq_unmask = pruss_intc_irq_unmask;
> +	irqchip->irq_retrigger = pruss_intc_irq_retrigger;
> +	irqchip->irq_request_resources = pruss_intc_irq_reqres;
> +	irqchip->irq_release_resources = pruss_intc_irq_relres;
> +	irqchip->name = dev_name(dev);

Should we also set `irqchip->parent_device = dev;` here?

I tried it and had to add pm runtime stuff as well, otherwise
requesting irqs would fail.

> +	intc->irqchip = irqchip;
> +
> +	/* always 64 events */
> +	intc->domain = irq_domain_add_linear(dev->of_node, MAX_PRU_SYS_EVENTS,
> +					     &pruss_intc_irq_domain_ops, intc);
> +	if (!intc->domain)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < MAX_NUM_HOST_IRQS; i++) {
> +		irq = platform_get_irq_byname(pdev, irq_names[i]);
> +		if (irq < 0) {
> +			dev_err(dev->parent, "platform_get_irq_byname failed for %s : %d\n",

Why dev->parent instead of just dev?

> +				irq_names[i], irq);
> +			goto fail_irq;
> +		}
> +
> +		intc->irqs[i] = irq;
> +		irq_set_handler_data(irq, intc);
> +		irq_set_chained_handler(irq, pruss_intc_irq_handler);
> +	}
> +
> +	return 0;
> +
> +fail_irq:
> +	while (--i >= 0) {
> +		if (intc->irqs[i])
> +			irq_set_chained_handler_and_data(intc->irqs[i], NULL,
> +							 NULL);
> +	}
> +	irq_domain_remove(intc->domain);
> +	return irq;
> +}
> +
> +static int pruss_intc_remove(struct platform_device *pdev)
> +{
> +	struct pruss_intc *intc = platform_get_drvdata(pdev);
> +	unsigned int hwirq;
> +	int i;
> +
> +	for (i = 0; i < MAX_NUM_HOST_IRQS; i++) {
> +		if (intc->irqs[i])
> +			irq_set_chained_handler_and_data(intc->irqs[i], NULL,
> +							 NULL);
> +	}
> +
> +	if (intc->domain) {

Is it actuall possible for intc->domain to be NULL here?

> +		for (hwirq = 0; hwirq < MAX_PRU_SYS_EVENTS; hwirq++)
> +			irq_dispose_mapping(irq_find_mapping(intc->domain,
> +							     hwirq));
> +		irq_domain_remove(intc->domain);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id pruss_intc_of_match[] = {
> +	{ .compatible = "ti,pruss-intc", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, pruss_intc_of_match);
> +
> +static struct platform_driver pruss_intc_driver = {
> +	.driver = {
> +		.name = "pruss-intc",
> +		.of_match_table = pruss_intc_of_match,
> +	},
> +	.probe  = pruss_intc_probe,
> +	.remove = pruss_intc_remove,
> +};
> +module_platform_driver(pruss_intc_driver);
> +
> +MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
> +MODULE_AUTHOR("Suman Anna <s-anna@ti.com>");
> +MODULE_DESCRIPTION("TI PRU-ICSS INTC Driver");
> +MODULE_LICENSE("GPL v2");
> 

^ permalink raw reply

* [RFC v2] DT-based tuner/demod init
From: Marc Gonzalez @ 2019-07-11 16:21 UTC (permalink / raw)
  To: linux-media, I2C, Linux ARM
  Cc: DT, Peter Korsgaard, Jeffrey Hugo, Wolfram Sang, Linus Walleij,
	Brad Love, Jonathan Neuschäfer, Bjorn Andersson,
	Antti Palosaari, Peter Rosin, Simon Horman, Mauro Carvalho Chehab,
	Olli Salonen

Hello everyone,

This is a follow-up RFC to my first request for comments:
"[RFC] SW connection between DVB Transport Stream demuxer and I2C-based frontend"
https://www.spinics.net/lists/arm-kernel/msg739972.html

Background: my SoC provides a "Transport Stream Interface" on-chip
(for which I wrote a small driver, tsif.c) as well as a tuner/demod combo
(si2141/si2168) on the board.

My original goal was: being able to link the tuner/demod from the device tree,
instead of hard-coding them in the TSIF driver.

(Please see the resulting code at the end of this message)

Aside from the tsif driver itself (I would appreciate if someone could answer
some questions and TODOs in that part as well), there were two hurdles that
had to be solved for this to work:

PROBLEM #1

The dvb_adapter and the tuner need to "see" the dvb_frontend struct filled
by the demod driver. In order to do this (in a generic, OO-like way), Brad
suggested that I could make 'struct dvb_frontend' the first field of a
demod's clientdata. That way, dvb_get_demod_fe() callers don't need to
know which demod they are tied to.

I'd like to hear comments on dvb_get_demod_fe() because
A. It is an essential part of the patch series, and
B. I have no better way to do this.


PROBLEM #2

Peter Rosin suggested a great way to get the demod instantiate the tuner
via DT (by using an i2c-gate node. I think we can all agree that this
solution is good to go?


So basically, I need an amen on solution for Problem #1, and ideally a few
comments on the driver itself. Then I can spin an actual patch series.

Regards.



diff --git a/arch/arm64/boot/dts/qcom/apq8098-batfish.dts b/arch/arm64/boot/dts/qcom/apq8098-batfish.dts
index 29d59ecad138..06cd2663e089 100644
--- a/arch/arm64/boot/dts/qcom/apq8098-batfish.dts
+++ b/arch/arm64/boot/dts/qcom/apq8098-batfish.dts
@@ -30,6 +30,48 @@
 	status = "ok";
 };
 
+&blsp1_i2c5 {
+	status = "ok";
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c5_default>;
+
+	demod: demod@64 {
+		compatible = "silabs,si2168";
+		reg = <0x64>;
+		reset-gpios = <&tlmm 84 GPIO_ACTIVE_LOW>;
+		i2c-gate {
+			tuner@60 {
+				compatible = "silabs,si2141";
+				reg = <0x60>;
+				demod = <&demod>;
+			};
+		};
+	};
+};
+
+&tlmm {
+	i2c5_default: i2c5-default {
+		pins = "gpio87", "gpio88";
+		function = "blsp_i2c5";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	tsif0_default: tsif0-default {
+		pins = "gpio89", "gpio90", "gpio91";
+		function = "tsif0";
+		drive-strength = <2>;
+		bias-pull-down;
+	};
+};
+
+&tsif {
+	demod = <&demod>;
+	pinctrl-0 = <&tsif0_default>;
+	pinctrl-names = "default";
+};
+
 &qusb2phy {
 	status = "ok";
 	vdda-pll-supply = <&vreg_l12a_1p8>;
diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index f8671a46392d..62518dcbe2a3 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -1206,6 +1206,16 @@
 			status = "disabled";
 		};
 
+		tsif: tsif@c1e7000 {
+			compatible = "qcom,msm8998-tsif";
+			reg = <0x0c1e7000 0x104>;
+			reg-names = "tsif0";
+			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "tsif0";
+			clocks = <&gcc GCC_TSIF_AHB_CLK>;
+			clock-names = "iface";
+		};
+
 		timer@17920000 {
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 6351a97f3d18..5a55c6bd0bd2 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -3030,3 +3030,17 @@ void dvb_frontend_detach(struct dvb_frontend *fe)
 	dvb_frontend_put(fe);
 }
 EXPORT_SYMBOL(dvb_frontend_detach);
+
+/*
+ * DT-enabled demodulator drivers are required to have 'struct dvb_frontend'
+ * as the first field of their state struct (stored as clientdata) in order
+ * to allow this function to return 'struct dvb_frontend' generically.
+ */
+struct dvb_frontend *dvb_get_demod_fe(struct device_node *np)
+{
+	struct device_node *demod_node = of_parse_phandle(np, "demod", 0);
+	struct i2c_client *demod = of_find_i2c_device_by_node(demod_node);
+	of_node_put(demod_node);
+	return demod ? i2c_get_clientdata(demod) : NULL;
+}
+EXPORT_SYMBOL(dvb_get_demod_fe);
diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index 48e8a376766e..09e1382ed128 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/delay.h>
+#include <linux/gpio/consumer.h>
 
 #include "si2168_priv.h"
 
@@ -663,6 +664,8 @@ static const struct dvb_frontend_ops si2168_ops = {
 static int si2168_probe(struct i2c_client *client,
 		const struct i2c_device_id *id)
 {
+	struct device *cdev = &client->dev;
+	struct si2168_config defconfig = { .ts_mode = SI2168_TS_SERIAL };
 	struct si2168_config *config = client->dev.platform_data;
 	struct si2168_dev *dev;
 	int ret;
@@ -670,6 +673,13 @@ static int si2168_probe(struct i2c_client *client,
 
 	dev_dbg(&client->dev, "\n");
 
+	if (cdev->of_node) {
+		struct gpio_desc *desc;
+		desc = devm_gpiod_get_optional(cdev, "reset", GPIOD_OUT_LOW);
+		if (IS_ERR(desc)) return PTR_ERR(desc);
+		config = &defconfig;
+	}
+
 	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
 	if (!dev) {
 		ret = -ENOMEM;
@@ -723,9 +733,21 @@ static int si2168_probe(struct i2c_client *client,
 	dev->version = (cmd.args[1]) << 24 | (cmd.args[3] - '0') << 16 |
 		       (cmd.args[4] - '0') << 8 | (cmd.args[5]) << 0;
 
+	/* create dvb_frontend */
+	memcpy(&dev->fe.ops, &si2168_ops, sizeof(si2168_ops));
+	dev->fe.demodulator_priv = client;
+	if (config->i2c_adapter)
+		*config->i2c_adapter = dev->muxc->adapter[0];
+	if (config->fe)
+		*config->fe = &dev->fe;
+	dev->ts_mode = config->ts_mode;
+	dev->ts_clock_inv = config->ts_clock_inv;
+	dev->ts_clock_gapped = config->ts_clock_gapped;
+	dev->spectral_inversion = config->spectral_inversion;
+
 	/* create mux i2c adapter for tuner */
 	dev->muxc = i2c_mux_alloc(client->adapter, &client->dev,
-				  1, 0, I2C_MUX_LOCKED,
+				  1, 0, I2C_MUX_LOCKED | I2C_MUX_GATE,
 				  si2168_select, si2168_deselect);
 	if (!dev->muxc) {
 		ret = -ENOMEM;
@@ -736,16 +758,6 @@ static int si2168_probe(struct i2c_client *client,
 	if (ret)
 		goto err_kfree;
 
-	/* create dvb_frontend */
-	memcpy(&dev->fe.ops, &si2168_ops, sizeof(struct dvb_frontend_ops));
-	dev->fe.demodulator_priv = client;
-	*config->i2c_adapter = dev->muxc->adapter[0];
-	*config->fe = &dev->fe;
-	dev->ts_mode = config->ts_mode;
-	dev->ts_clock_inv = config->ts_clock_inv;
-	dev->ts_clock_gapped = config->ts_clock_gapped;
-	dev->spectral_inversion = config->spectral_inversion;
-
 	dev_info(&client->dev, "Silicon Labs Si2168-%c%d%d successfully identified\n",
 		 dev->version >> 24 & 0xff, dev->version >> 16 & 0xff,
 		 dev->version >> 8 & 0xff);
diff --git a/drivers/media/dvb-frontends/si2168_priv.h b/drivers/media/dvb-frontends/si2168_priv.h
index 804d5b30c697..1e3cac3b8381 100644
--- a/drivers/media/dvb-frontends/si2168_priv.h
+++ b/drivers/media/dvb-frontends/si2168_priv.h
@@ -22,9 +22,9 @@
 
 /* state struct */
 struct si2168_dev {
+	struct dvb_frontend fe; /* see dvb_get_demod_fe() */
 	struct mutex i2c_mutex;
 	struct i2c_mux_core *muxc;
-	struct dvb_frontend fe;
 	enum fe_delivery_system delivery_system;
 	enum fe_status fe_status;
 	#define SI2168_CHIP_ID_A20 ('A' << 24 | 68 << 16 | '2' << 8 | '0' << 0)
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 7cbbd925124c..6a522e647a9d 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -101,3 +101,5 @@ obj-y					+= meson/
 obj-y					+= cros-ec-cec/
 
 obj-$(CONFIG_VIDEO_SUN6I_CSI)		+= sunxi/sun6i-csi/
+
+obj-y += tsif.o
diff --git a/drivers/media/platform/tsif.c b/drivers/media/platform/tsif.c
new file mode 100644
index 000000000000..b136f334e9c6
--- /dev/null
+++ b/drivers/media/platform/tsif.c
@@ -0,0 +1,185 @@
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <media/dvb_frontend.h>
+#include <media/dvb_demux.h>
+#include <media/dmxdev.h>
+
+/* TSIF register offsets */
+#define TSIF_STS_CTL	0x0	/* status and control */
+#define TSIF_DATA_PORT	0x100
+
+/* TSIF_STS_CTL bits */
+#define ENABLE_IRQ	BIT(28)
+#define TSIF_STOP	BIT(3)
+#define TSIF_START	BIT(0)
+
+struct tsif {
+	void __iomem *base;
+	struct clk *clk;
+	int ref_count; /*** TODO: use atomic_t ??? or refcount_t ??? or kref ??? ***/
+	u32 buf[48];
+	struct dvb_frontend *fe;
+	/*** DO I NEED ALL 4 ***/
+	//struct dmx_frontend dmx_frontend;
+	struct dvb_adapter dvb_adapter;
+	struct dvb_demux dvb_demux;
+	struct dmxdev dmxdev;
+};
+
+static int start_tsif(struct dvb_demux_feed *feed)
+{
+	struct tsif *tsif = feed->demux->priv;
+	printk("%s: feed PID=%u\n", __func__, feed->pid);
+
+	if (tsif->ref_count++ == 0) {
+		u32 val = TSIF_START | ENABLE_IRQ | BIT(29);
+		writel_relaxed(val, tsif->base + TSIF_STS_CTL);
+	}
+
+	return 0;
+}
+
+static int stop_tsif(struct dvb_demux_feed *feed)
+{
+	struct tsif *tsif = feed->demux->priv;
+	printk("%s: feed PID=%u\n", __func__, feed->pid);
+
+	if (--tsif->ref_count == 0) {
+		writel_relaxed(TSIF_STOP, tsif->base + TSIF_STS_CTL);
+	}
+
+	return 0;
+}
+
+static irqreturn_t tsif_isr(int irq, void *arg)
+{
+	int i;
+	u32 status;
+	struct tsif *tsif = arg;
+
+	status = readl_relaxed(tsif->base + TSIF_STS_CTL);
+	writel_relaxed(status, tsif->base + TSIF_STS_CTL);
+
+	for (i = 0; i < 48; ++i)
+		tsif->buf[i] = readl_relaxed(tsif->base + TSIF_DATA_PORT);
+
+	dvb_dmx_swfilter_packets(&tsif->dvb_demux, (void *)tsif->buf, 1);
+
+	return IRQ_HANDLED;
+}
+
+static int tsif_probe(struct platform_device *pdev)
+{
+	int err, virq;
+	struct tsif *tsif;
+	struct resource *res;
+	struct device *dev = &pdev->dev;
+
+	tsif = devm_kzalloc(dev, sizeof(*tsif), GFP_KERNEL);
+	if (!tsif)
+		return -ENOMEM;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "tsif0");
+	tsif->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(tsif->base))
+		return PTR_ERR(tsif->base);
+
+	virq = platform_get_irq_byname(pdev, "tsif0");
+	err = devm_request_irq(dev, virq, tsif_isr, IRQF_SHARED, "tsif", tsif);
+	if (err)
+		return err;
+
+	tsif->clk = devm_clk_get(dev, "iface");
+	if (IS_ERR(tsif->clk))
+		return PTR_ERR(tsif->clk);
+
+	tsif->fe = dvb_get_demod_fe(dev->of_node);
+	if (!tsif->fe)
+		return -ENXIO;
+
+	/*** TODO: use devm version ***/
+	clk_prepare_enable(tsif->clk);
+
+	{
+	int ret;
+	short any = DVB_UNSET;
+
+	ret = dvb_register_adapter(&tsif->dvb_adapter, "tsif", THIS_MODULE, dev, &any);
+	if (ret < 0) panic("dvb_register_adapter");
+
+	tsif->dvb_demux.priv = tsif;
+	/* Not sure the diff between filter and feed? */
+	tsif->dvb_demux.filternum = 16; /*** Dunno what to put here ***/
+	tsif->dvb_demux.feednum = 16;	/*** Dunno what to put here ***/
+	tsif->dvb_demux.start_feed = start_tsif;
+	tsif->dvb_demux.stop_feed = stop_tsif;
+
+	ret = dvb_dmx_init(&tsif->dvb_demux);
+	if (ret < 0) panic("dvb_dmx_init");
+
+	/* What relation to dvb_demux.filternum??? */
+	/* Do I need this object?? */
+	tsif->dmxdev.filternum = 16;
+	tsif->dmxdev.demux = &tsif->dvb_demux.dmx;
+
+	ret = dvb_dmxdev_init(&tsif->dmxdev, &tsif->dvb_adapter);
+	if (ret < 0) panic("dvb_dmxdev_init");
+
+#if 0
+	/*** These calls don't seem required??? ***/
+	/*** Who reads this? Do I need to set it? ***/
+	tsif->dmx_frontend.source = DMX_FRONTEND_0;
+
+	/* Required or done elsewhere? */
+	ret = tsif->dvb_demux.dmx.add_frontend(&tsif->dvb_demux.dmx, &tsif->dmx_frontend);
+	if (ret < 0) panic("add_frontend");
+
+	/* Required or done elsewhere? */
+	ret = tsif->dvb_demux.dmx.connect_frontend(&tsif->dvb_demux.dmx, &tsif->dmx_frontend);
+	if (ret < 0) panic("connect_frontend");
+#endif
+
+	ret = dvb_register_frontend(&tsif->dvb_adapter, tsif->fe);
+	if (ret < 0) panic("dvb_register_frontend");
+	}
+
+	platform_set_drvdata(pdev, tsif);
+	return 0;
+}
+
+/*** TODO: Double check .remove callback ***/
+static int tsif_remove(struct platform_device *pdev)
+{
+	struct tsif *tsif = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(tsif->clk);
+
+	dvb_unregister_frontend(tsif->fe);
+	//tsif->dvb_demux.dmx.remove_frontend(&tsif->dvb_demux.dmx, &tsif->dmx_frontend);
+	dvb_dmxdev_release(&tsif->dmxdev);
+	dvb_dmx_release(&tsif->dvb_demux);
+	dvb_unregister_adapter(&tsif->dvb_adapter);
+
+	return 0;
+}
+
+static const struct of_device_id tsif_of_ids[] = {
+	{ .compatible = "qcom,msm8998-tsif" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tsif_of_ids);
+
+static struct platform_driver qcom_tsif_driver = {
+	.probe  = tsif_probe,
+	.remove = tsif_remove,
+	.driver = {
+		.name = "qcom-tsif",
+		.of_match_table = tsif_of_ids,
+	},
+};
+
+module_platform_driver(qcom_tsif_driver);
+
+MODULE_DESCRIPTION("Qualcomm TSIF driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 7be893def190..9d2ca775ada9 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -420,12 +420,18 @@ static void si2157_stat_work(struct work_struct *work)
 static int si2157_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
+	struct device *cdev = &client->dev;
+	struct si2157_config defconfig = { 0 };
 	struct si2157_config *cfg = client->dev.platform_data;
-	struct dvb_frontend *fe = cfg->fe;
 	struct si2157_dev *dev;
 	struct si2157_cmd cmd;
 	int ret;
 
+	if (cdev->of_node) {
+		cfg = &defconfig;
+		cfg->fe = dvb_get_demod_fe(cdev->of_node);
+	}
+
 	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
 	if (!dev) {
 		ret = -ENOMEM;
@@ -449,8 +455,8 @@ static int si2157_probe(struct i2c_client *client,
 	if (ret)
 		goto err_kfree;
 
-	memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
-	fe->tuner_priv = client;
+	memcpy(&dev->fe->ops.tuner_ops, &si2157_ops, sizeof(si2157_ops));
+	dev->fe->tuner_priv = client;
 
 #ifdef CONFIG_MEDIA_CONTROLLER
 	if (cfg->mdev) {
diff --git a/include/media/dvb_frontend.h b/include/media/dvb_frontend.h
index f05cd7b94a2c..f8ea4839095f 100644
--- a/include/media/dvb_frontend.h
+++ b/include/media/dvb_frontend.h
@@ -821,4 +821,6 @@ void dvb_frontend_reinitialise(struct dvb_frontend *fe);
  */
 void dvb_frontend_sleep_until(ktime_t *waketime, u32 add_usec);
 
+struct dvb_frontend *dvb_get_demod_fe(struct device_node *np);
+
 #endif

^ permalink raw reply related

* Re: [PATCH 5/8] dt-bindings: PCI: Add Amazon's Annapurna Labs PCIe host bridge binding
From: Chocron, Jonathan @ 2019-07-11 15:44 UTC (permalink / raw)
  To: Shenhar, Talel, lorenzo.pieralisi@arm.com
  Cc: linux-kernel@vger.kernel.org, robh+dt@kernel.org,
	jingoohan1@gmail.com, Woodhouse, David, Hanoch, Uri,
	devicetree@vger.kernel.org, gustavo.pimentel@synopsys.com,
	Wasserstrom, Barak, Saidi, Ali, mark.rutland@arm.com, Hawa, Hanna,
	Krupnik, Ronen, bhelgaas@google.com, linux-pci@vger.kernel.org,
	benh@kernel.crashing.org
In-Reply-To: <20190711093210.GA26088@e121166-lin.cambridge.arm.com>

On Thu, 2019-07-11 at 10:32 +0100, Lorenzo Pieralisi wrote:
> On Thu, Jul 11, 2019 at 10:12:35AM +0300, Shenhar, Talel wrote:
> > 
> > On 7/10/2019 7:45 PM, Jonathan Chocron wrote:
> > > Document Amazon's Annapurna Labs PCIe host bridge.
> > 
> > That is the way! (best to keep same wordings (Amazon's)
> 
> Guys,
> 
> as a heads-up, the original posting, for whatever reason, did not hit
> linux-pci@vger, which means that from a PCI patches queue review
> point
> of view it does not exist.
> 
> It ought to be fixed otherwise we won't be able to review the code.
> 
> Lorenzo
> 

I've succeeded to redirect the original emails to linux-pci@vger, so
please proceed with reviewing the patches.

Thanks,
   Jonathan

^ permalink raw reply

* [PATCH 12/12] arm64: dts: qcom: Enable Q6v5 WCSS for ipq8074 SoC
From: Gokul Sriram Palanisamy @ 2019-07-11 15:41 UTC (permalink / raw)
  To: agross, david.brown, robh+dt, mark.rutland, mturquette, sboyd,
	jassisinghbrar, ohad, bjorn.andersson, linux-arm-msm, devicetree,
	linux-kernel, linux-clk, linux-remoteproc, sricharan, gokulsri
In-Reply-To: <1562859668-14209-1-git-send-email-gokulsri@codeaurora.org>

Enable remoteproc WCSS PIL driver with glink
and ssr subdevices. Also configures shared memory
and enables smp2p and mailboxes required for IPC.

Signed-off-by: Gokul Sriram Palanisamy <gokulsri@codeaurora.org>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
Signed-off-by: Nikhil Prakash V <nprakash@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 125 ++++++++++++++++++++++++++++++++++
 1 file changed, 125 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index 6a61a63..c24e3f6 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -10,6 +10,22 @@
 	model = "Qualcomm Technologies, Inc. IPQ8074";
 	compatible = "qcom,ipq8074";
 
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		smem_region:smem@4ab00000 {
+			no-map;
+			reg = <0x0 0x4ab00000 0x0 0x00100000>;
+		};
+
+		q6_region: q6@4b000000 {
+			no-map;
+			reg = <0x0 0x4b000000 0x0 0x05f00000>;
+		};
+	};
+
 	firmware {
 		scm {
 			compatible = "qcom,scm-ipq8074", "qcom,scm";
@@ -431,6 +447,115 @@
 				      "axi_m_sticky";
 			status = "disabled";
 		};
+		apcs: syscon@b111000 {
+			compatible = "syscon";
+			reg = <0x0B111000 0x1000>;
+		};
+
+		wcss: smp2p-wcss {
+			compatible = "qcom,smp2p";
+			qcom,smem = <435>, <428>;
+
+			interrupt-parent = <&intc>;
+			interrupts = <0 322 1>;
+
+			qcom,ipc = <&apcs 8 9>;
+
+			qcom,local-pid = <0>;
+			qcom,remote-pid = <1>;
+
+			wcss_smp2p_out: master-kernel {
+				qcom,entry-name = "master-kernel";
+				qcom,smp2p-feature-ssr-ack;
+				#qcom,smem-state-cells = <1>;
+			};
+
+			wcss_smp2p_in: slave-kernel {
+				qcom,entry-name = "slave-kernel";
+
+				interrupt-controller;
+				#interrupt-cells = <2>;
+			};
+		};
+
+		tcsr_q6_block: syscon@1945000 {
+			compatible = "syscon";
+			reg = <0x1945000 0xE000>;
+		};
+
+		tcsr_mutex_block: syscon@193d000 {
+			compatible = "syscon";
+			reg = <0x1905000 0x8000>;
+		};
+
+		tcsr_mutex: hwlock@193d000 {
+			compatible = "qcom,tcsr-mutex";
+			syscon = <&tcsr_mutex_block 0 0x80>;
+			#hwlock-cells = <1>;
+		};
+
+		smem: qcom,smem@4AB00000 {
+			compatible = "qcom,smem";
+			memory-region = <&smem_region>;
+			hwlocks = <&tcsr_mutex 0>;
+		};
+
+		apcs_glb: mailbox@b111000 {
+			compatible = "qcom,ipq8074-apcs-apps-global";
+			reg = <0xb111000 0x1000>;
+
+			#mbox-cells = <1>;
+		};
+
+		q6v5_wcss: q6v5_wcss@CD00000 {
+			compatible = "qcom,ipq8074-wcss-pil";
+			reg = <0xCD00000 0x4040>,
+			      <0x4AB000 0x20>;
+			reg-names = "qdsp6",
+				    "rmb";
+			qca,auto-restart;
+			qca,extended-intc;
+			interrupts-extended = <&intc 0 325 1>,
+					      <&wcss_smp2p_in 0 0>,
+					      <&wcss_smp2p_in 1 0>,
+					      <&wcss_smp2p_in 2 0>,
+					      <&wcss_smp2p_in 3 0>;
+			interrupt-names = "wdog",
+					  "fatal",
+					  "ready",
+					  "handover",
+					  "stop-ack";
+
+			resets = <&gcc GCC_WCSSAON_RESET>,
+				 <&gcc GCC_WCSS_BCR>,
+				 <&gcc GCC_WCSS_Q6_BCR>;
+
+			reset-names = "wcss_aon_reset",
+				      "wcss_reset",
+				      "wcss_q6_reset";
+
+			clocks = <&gcc GCC_PRNG_AHB_CLK>;
+			clock-names = "prng";
+
+			qcom,halt-regs = <&tcsr_q6_block 0xA000 0xD000 0x0>;
+
+			qcom,smem-states = <&wcss_smp2p_out 0>,
+					   <&wcss_smp2p_out 1>;
+			qcom,smem-state-names = "shutdown",
+						"stop";
+
+			memory-region = <&q6_region>;
+
+			glink-edge {
+				interrupts = <GIC_SPI 321 IRQ_TYPE_EDGE_RISING>;
+				qcom,remote-pid = <1>;
+				mboxes = <&apcs_glb 8>;
+
+				rpm_requests {
+					qcom,glink-channels = "IPCRTR";
+				};
+			};
+		};
 	};
 
 	cpus {
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH 11/12] arm64: dts: Add support for scm on IPQ8074 SoCs
From: Gokul Sriram Palanisamy @ 2019-07-11 15:41 UTC (permalink / raw)
  To: agross, david.brown, robh+dt, mark.rutland, mturquette, sboyd,
	jassisinghbrar, ohad, bjorn.andersson, linux-arm-msm, devicetree,
	linux-kernel, linux-clk, linux-remoteproc, sricharan, gokulsri
In-Reply-To: <1562859668-14209-1-git-send-email-gokulsri@codeaurora.org>

Enables scm support, clock is not needed for enabling scm interface.

Signed-off-by: Gokul Sriram Palanisamy <gokulsri@codeaurora.org>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index 67ee5f5..6a61a63 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -10,6 +10,12 @@
 	model = "Qualcomm Technologies, Inc. IPQ8074";
 	compatible = "qcom,ipq8074";
 
+	firmware {
+		scm {
+			compatible = "qcom,scm-ipq8074", "qcom,scm";
+		};
+	};
+
 	soc: soc {
 		#address-cells = <0x1>;
 		#size-cells = <0x1>;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH 10/12] dt-bindings: firmware: qcom: Add compatible for IPQ8074 SoC
From: Gokul Sriram Palanisamy @ 2019-07-11 15:41 UTC (permalink / raw)
  To: agross, david.brown, robh+dt, mark.rutland, mturquette, sboyd,
	jassisinghbrar, ohad, bjorn.andersson, linux-arm-msm, devicetree,
	linux-kernel, linux-clk, linux-remoteproc, sricharan, gokulsri
In-Reply-To: <1562859668-14209-1-git-send-email-gokulsri@codeaurora.org>

Add compatible for IPQ8074 support.
This does not need clocks for scm calls.

Signed-off-by: Gokul Sriram Palanisamy <gokulsri@codeaurora.org>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 Documentation/devicetree/bindings/firmware/qcom,scm.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.txt b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
index 41f133a..3b153c1 100644
--- a/Documentation/devicetree/bindings/firmware/qcom,scm.txt
+++ b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
@@ -17,6 +17,7 @@ Required properties:
  * "qcom,scm-msm8998"
  * "qcom,scm-ipq4019"
  * "qcom,scm-sdm845"
+ * "qcom,scm-ipq8074"
  and:
  * "qcom,scm"
 - clocks: Specifies clocks needed by the SCM interface, if any:
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH 09/12] mailbox: qcom: Add support for IPQ8074 APCS
From: Gokul Sriram Palanisamy @ 2019-07-11 15:41 UTC (permalink / raw)
  To: agross, david.brown, robh+dt, mark.rutland, mturquette, sboyd,
	jassisinghbrar, ohad, bjorn.andersson, linux-arm-msm, devicetree,
	linux-kernel, linux-clk, linux-remoteproc, sricharan, gokulsri
In-Reply-To: <1562859668-14209-1-git-send-email-gokulsri@codeaurora.org>

Add support of IPQ8074 with IPC register offset as 8.

Signed-off-by: Gokul Sriram Palanisamy <gokulsri@codeaurora.org>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 drivers/mailbox/qcom-apcs-ipc-mailbox.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
index 705e17a..52185eb 100644
--- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
+++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
@@ -119,6 +119,7 @@ static int qcom_apcs_ipc_remove(struct platform_device *pdev)
 	{ .compatible = "qcom,msm8998-apcs-hmss-global", .data = (void *)8 },
 	{ .compatible = "qcom,qcs404-apcs-apps-global", .data = (void *)8 },
 	{ .compatible = "qcom,sdm845-apss-shared", .data = (void *)12 },
+	{ .compatible = "qcom,ipq8074-apcs-apps-global", .data = (void *)8 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, qcom_apcs_ipc_of_match);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH 08/12] dt-bindings: mailbox: qom: Add ipq8074 APPS compatible
From: Gokul Sriram Palanisamy @ 2019-07-11 15:41 UTC (permalink / raw)
  To: agross, david.brown, robh+dt, mark.rutland, mturquette, sboyd,
	jassisinghbrar, ohad, bjorn.andersson, linux-arm-msm, devicetree,
	linux-kernel, linux-clk, linux-remoteproc, sricharan, gokulsri
In-Reply-To: <1562859668-14209-1-git-send-email-gokulsri@codeaurora.org>

Add mailbox support required in IPQ8074 SoCs.

Signed-off-by: Gokul Sriram Palanisamy <gokulsri@codeaurora.org>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
index 1232fc9..389c6ce 100644
--- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
+++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
@@ -13,6 +13,7 @@ platforms.
 		    "qcom,msm8998-apcs-hmss-global"
 		    "qcom,qcs404-apcs-apps-global"
 		    "qcom,sdm845-apss-shared"
+		    "qcom,ipq8074-apcs-apps-global"
 
 - reg:
 	Usage: required
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH 07/12] clk: qcom: Add WCSSAON reset
From: Gokul Sriram Palanisamy @ 2019-07-11 15:41 UTC (permalink / raw)
  To: agross, david.brown, robh+dt, mark.rutland, mturquette, sboyd,
	jassisinghbrar, ohad, bjorn.andersson, linux-arm-msm, devicetree,
	linux-kernel, linux-clk, linux-remoteproc, sricharan, gokulsri
In-Reply-To: <1562859668-14209-1-git-send-email-gokulsri@codeaurora.org>

Add WCSSAON reset required for Q6v5 on IPQ8074 SoC.

Signed-off-by: Gokul Sriram Palanisamy <gokulsri@codeaurora.org>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
Signed-off-by: Nikhil Prakash V <nprakash@codeaurora.org>
---
 drivers/clk/qcom/gcc-ipq8074.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/qcom/gcc-ipq8074.c b/drivers/clk/qcom/gcc-ipq8074.c
index 39ade58..06092f8 100644
--- a/drivers/clk/qcom/gcc-ipq8074.c
+++ b/drivers/clk/qcom/gcc-ipq8074.c
@@ -4685,6 +4685,7 @@ enum {
 	[GCC_PCIE1_AXI_SLAVE_ARES] = { 0x76040, 4 },
 	[GCC_PCIE1_AHB_ARES] = { 0x76040, 5 },
 	[GCC_PCIE1_AXI_MASTER_STICKY_ARES] = { 0x76040, 6 },
+	[GCC_WCSSAON_RESET] = { 0x59010, 0 },
 };
 
 static const struct of_device_id gcc_ipq8074_match_table[] = {
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH 06/12] dt-bindings: clock: qcom: Add reset for WCSSAON
From: Gokul Sriram Palanisamy @ 2019-07-11 15:41 UTC (permalink / raw)
  To: agross, david.brown, robh+dt, mark.rutland, mturquette, sboyd,
	jassisinghbrar, ohad, bjorn.andersson, linux-arm-msm, devicetree,
	linux-kernel, linux-clk, linux-remoteproc, sricharan, gokulsri
In-Reply-To: <1562859668-14209-1-git-send-email-gokulsri@codeaurora.org>

Add binding for WCSSAON reset required for Q6v5 reset on IPQ8074 SoC.

Signed-off-by: Gokul Sriram Palanisamy <gokulsri@codeaurora.org>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
Signed-off-by: Nikhil Prakash V <nprakash@codeaurora.org>
---
 include/dt-bindings/clock/qcom,gcc-ipq8074.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/clock/qcom,gcc-ipq8074.h b/include/dt-bindings/clock/qcom,gcc-ipq8074.h
index 4de4811..04e1f57 100644
--- a/include/dt-bindings/clock/qcom,gcc-ipq8074.h
+++ b/include/dt-bindings/clock/qcom,gcc-ipq8074.h
@@ -362,5 +362,6 @@
 #define GCC_PCIE1_AXI_SLAVE_ARES		128
 #define GCC_PCIE1_AHB_ARES			129
 #define GCC_PCIE1_AXI_MASTER_STICKY_ARES	130
+#define GCC_WCSSAON_RESET			131
 
 #endif
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH 05/12] remoteproc: qcom: Update regmap offsets for halt register
From: Gokul Sriram Palanisamy @ 2019-07-11 15:41 UTC (permalink / raw)
  To: agross, david.brown, robh+dt, mark.rutland, mturquette, sboyd,
	jassisinghbrar, ohad, bjorn.andersson, linux-arm-msm, devicetree,
	linux-kernel, linux-clk, linux-remoteproc, sricharan, gokulsri
In-Reply-To: <1562859668-14209-1-git-send-email-gokulsri@codeaurora.org>

Fixed issue in reading halt-regs parameter from device-tree.
Also adding a flag to check for bcr reset which is not
required for ipq8074.

Signed-off-by: Gokul Sriram Palanisamy <gokulsri@codeaurora.org>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 drivers/remoteproc/qcom_q6v5_wcss.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index 6d3ef06..b4ef5d3 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -83,7 +83,7 @@
 #define TCSR_WCSS_CLK_MASK	0x1F
 #define TCSR_WCSS_CLK_ENABLE	0x14
 
-#define MAX_HALT_REG		3
+#define MAX_HALT_REG		4
 
 #define WCNSS_PAS_ID		6
 
@@ -149,6 +149,7 @@ struct wcss_data {
 	int crash_reason_smem;
 	u8 version;
 	bool aon_reset_required;
+	bool bcr_reset_required;
 	const char *ssr_name;
 	const char *sysmon_name;
 	int ssctl_id;
@@ -810,7 +811,8 @@ static int q6v5_wcss_load(struct rproc *rproc, const struct firmware *fw)
 };
 
 static int q6v5_wcss_init_reset(struct q6v5_wcss *wcss,
-				bool aon_reset_required)
+				bool aon_reset_required,
+				bool bcr_reset_required)
 {
 	struct device *dev = wcss->dev;
 
@@ -834,10 +836,13 @@ static int q6v5_wcss_init_reset(struct q6v5_wcss *wcss,
 		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_reset);
+	if (bcr_reset_required) {
+		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_reset);
+		}
 	}
 
 	return 0;
@@ -885,9 +890,9 @@ static int q6v5_wcss_init_mmio(struct q6v5_wcss *wcss,
 		return -EINVAL;
 	}
 
-	wcss->halt_q6 = halt_reg[0];
-	wcss->halt_wcss = halt_reg[1];
-	wcss->halt_nc = halt_reg[2];
+	wcss->halt_q6 = halt_reg[1];
+	wcss->halt_wcss = halt_reg[2];
+	wcss->halt_nc = halt_reg[3];
 
 	return 0;
 }
@@ -1111,7 +1116,8 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
 			goto free_rproc;
 	}
 
-	ret = q6v5_wcss_init_reset(wcss, desc->aon_reset_required);
+	ret = q6v5_wcss_init_reset(wcss, desc->aon_reset_required,
+				   desc->bcr_reset_required);
 	if (ret)
 		goto free_rproc;
 
@@ -1156,6 +1162,7 @@ static int q6v5_wcss_remove(struct platform_device *pdev)
 	.m3_firmware_name = "IPQ8074/m3_fw.mdt",
 	.crash_reason_smem = 421,
 	.aon_reset_required = true,
+	.bcr_reset_required = false,
 	.ssr_name = "q6wcss",
 	.ops = &q6v5_wcss_ipq8074_ops,
 	.requires_force_stop = true,
@@ -1169,6 +1176,7 @@ static int q6v5_wcss_remove(struct platform_device *pdev)
 	.q6_firmware_name = "wcnss.mdt",
 	.version = WCSS_QCS404,
 	.aon_reset_required = false,
+	.bcr_reset_required = true,
 	.ssr_name = "mpss",
 	.sysmon_name = "wcnss",
 	.ssctl_id = 0x12,
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH 04/12] remoteproc: qcom: Add ssr subdevice identifier
From: Gokul Sriram Palanisamy @ 2019-07-11 15:41 UTC (permalink / raw)
  To: agross, david.brown, robh+dt, mark.rutland, mturquette, sboyd,
	jassisinghbrar, ohad, bjorn.andersson, linux-arm-msm, devicetree,
	linux-kernel, linux-clk, linux-remoteproc, sricharan, gokulsri
In-Reply-To: <1562859668-14209-1-git-send-email-gokulsri@codeaurora.org>

Add name for ssr subdevice on IPQ8074 SoC.

Signed-off-by: Gokul Sriram Palanisamy <gokulsri@codeaurora.org>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
Signed-off-by: Nikhil Prakash V <nprakash@codeaurora.org>
---
 drivers/remoteproc/qcom_q6v5_wcss.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index 5957114..6d3ef06 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -1156,6 +1156,7 @@ static int q6v5_wcss_remove(struct platform_device *pdev)
 	.m3_firmware_name = "IPQ8074/m3_fw.mdt",
 	.crash_reason_smem = 421,
 	.aon_reset_required = true,
+	.ssr_name = "q6wcss",
 	.ops = &q6v5_wcss_ipq8074_ops,
 	.requires_force_stop = true,
 	.need_mem_protection = true,
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH 03/12] remoteproc: qcom: Add support for split q6 + m3 wlan firmware
From: Gokul Sriram Palanisamy @ 2019-07-11 15:40 UTC (permalink / raw)
  To: agross, david.brown, robh+dt, mark.rutland, mturquette, sboyd,
	jassisinghbrar, ohad, bjorn.andersson, linux-arm-msm, devicetree,
	linux-kernel, linux-clk, linux-remoteproc, sricharan, gokulsri
In-Reply-To: <1562859668-14209-1-git-send-email-gokulsri@codeaurora.org>

IPQ8074 supports split firmware for q6 and m3 as well.
So add support for loading the m3 firmware before q6.
Now the drivers works fine for both split and unified
firmwares.

Signed-off-by: Gokul Sriram Palanisamy <gokulsri@codeaurora.org>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
Signed-off-by: Nikhil Prakash V <nprakash@codeaurora.org>
---
 drivers/remoteproc/qcom_q6v5_wcss.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index 8418f57..5957114 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -138,12 +138,14 @@ struct q6v5_wcss {
 	u8 version;
 	bool requires_force_stop;
 	bool need_mem_protection;
+	const char *m3_firmware_name;
 };
 
 struct wcss_data {
 	int (*init_clock)(struct q6v5_wcss *wcss);
 	int (*init_regulator)(struct q6v5_wcss *wcss);
-	const char *firmware_name;
+	const char *q6_firmware_name;
+	const char *m3_firmware_name;
 	int crash_reason_smem;
 	u8 version;
 	bool aon_reset_required;
@@ -759,7 +761,26 @@ static void *q6v5_wcss_da_to_va(struct rproc *rproc, u64 da, int len)
 static int q6v5_wcss_load(struct rproc *rproc, const struct firmware *fw)
 {
 	struct q6v5_wcss *wcss = rproc->priv;
+	const struct firmware *m3_fw;
+	int ret;
+
+	if (wcss->m3_firmware_name) {
+		ret = request_firmware(&m3_fw, wcss->m3_firmware_name,
+				       wcss->dev);
+		if (ret)
+			goto skip_m3;
+
+		ret = qcom_mdt_load_no_init(wcss->dev, m3_fw,
+					    wcss->m3_firmware_name, 0,
+					    wcss->mem_region, wcss->mem_phys,
+					    wcss->mem_size, &wcss->mem_reloc);
+		if (ret) {
+			dev_err(wcss->dev, "can't load m3_fw.bXX\n");
+			return ret;
+		}
+	}
 
+skip_m3:
 	if (wcss->need_mem_protection)
 		return qcom_mdt_load(wcss->dev, fw, rproc->firmware,
 				     WCNSS_PAS_ID, wcss->mem_region,
@@ -1055,7 +1076,7 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
 		return -EPROBE_DEFER;
 
 	rproc = rproc_alloc(&pdev->dev, pdev->name, desc->ops,
-			    desc->firmware_name, sizeof(*wcss));
+			    desc->q6_firmware_name, sizeof(*wcss));
 	if (!rproc) {
 		dev_err(&pdev->dev, "failed to allocate rproc\n");
 		return -ENOMEM;
@@ -1068,6 +1089,7 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
 	wcss->version = desc->version;
 	wcss->requires_force_stop = desc->requires_force_stop;
 	wcss->need_mem_protection = desc->need_mem_protection;
+	wcss->m3_firmware_name = desc->m3_firmware_name;
 
 	ret = q6v5_wcss_init_mmio(wcss, pdev);
 	if (ret)
@@ -1130,7 +1152,8 @@ static int q6v5_wcss_remove(struct platform_device *pdev)
 
 static const struct wcss_data wcss_ipq8074_res_init = {
 	.init_clock = ipq8074_init_clock,
-	.firmware_name = "IPQ8074/q6_fw.mdt",
+	.q6_firmware_name = "IPQ8074/q6_fw.mdt",
+	.m3_firmware_name = "IPQ8074/m3_fw.mdt",
 	.crash_reason_smem = 421,
 	.aon_reset_required = true,
 	.ops = &q6v5_wcss_ipq8074_ops,
@@ -1142,7 +1165,7 @@ static int q6v5_wcss_remove(struct platform_device *pdev)
 	.init_clock = qcs404_init_clock,
 	.init_regulator = qcs404_init_regulator,
 	.crash_reason_smem = 421,
-	.firmware_name = "wcnss.mdt",
+	.q6_firmware_name = "wcnss.mdt",
 	.version = WCSS_QCS404,
 	.aon_reset_required = false,
 	.ssr_name = "mpss",
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH 02/12] remoteproc: qcom: Add secure PIL support
From: Gokul Sriram Palanisamy @ 2019-07-11 15:40 UTC (permalink / raw)
  To: agross, david.brown, robh+dt, mark.rutland, mturquette, sboyd,
	jassisinghbrar, ohad, bjorn.andersson, linux-arm-msm, devicetree,
	linux-kernel, linux-clk, linux-remoteproc, sricharan, gokulsri
In-Reply-To: <1562859668-14209-1-git-send-email-gokulsri@codeaurora.org>

IPQ8074 uses secure PIL. Hence, adding the support for the same.

Signed-off-by: Gokul Sriram Palanisamy <gokulsri@codeaurora.org>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
Signed-off-by: Nikhil Prakash V <nprakash@codeaurora.org>
---
 drivers/remoteproc/qcom_q6v5_wcss.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index dc2bae4..8418f57 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -19,6 +19,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 #include <linux/soc/qcom/mdt_loader.h>
+#include <linux/qcom_scm.h>
 #include "qcom_common.h"
 #include "qcom_q6v5.h"
 
@@ -83,6 +84,9 @@
 #define TCSR_WCSS_CLK_ENABLE	0x14
 
 #define MAX_HALT_REG		3
+
+#define WCNSS_PAS_ID		6
+
 enum {
 	WCSS_IPQ8074,
 	WCSS_QCS404,
@@ -133,6 +137,7 @@ struct q6v5_wcss {
 	int crash_reason_smem;
 	u8 version;
 	bool requires_force_stop;
+	bool need_mem_protection;
 };
 
 struct wcss_data {
@@ -147,6 +152,7 @@ struct wcss_data {
 	int ssctl_id;
 	const struct rproc_ops *ops;
 	bool requires_force_stop;
+	bool need_mem_protection;
 };
 
 static int q6v5_wcss_reset(struct q6v5_wcss *wcss)
@@ -246,6 +252,15 @@ static int q6v5_wcss_start(struct rproc *rproc)
 
 	qcom_q6v5_prepare(&wcss->q6v5);
 
+	if (wcss->need_mem_protection) {
+		ret = qcom_scm_pas_auth_and_reset(WCNSS_PAS_ID);
+		if (ret) {
+			dev_err(wcss->dev, "wcss_reset failed\n");
+			return ret;
+		}
+		goto wait_for_reset;
+	}
+
 	/* Release Q6 and WCSS reset */
 	ret = reset_control_deassert(wcss->wcss_reset);
 	if (ret) {
@@ -280,6 +295,7 @@ static int q6v5_wcss_start(struct rproc *rproc)
 	if (ret)
 		goto wcss_q6_reset;
 
+wait_for_reset:
 	ret = qcom_q6v5_wait_for_start(&wcss->q6v5, 5 * HZ);
 	if (ret == -ETIMEDOUT)
 		dev_err(wcss->dev, "start timed out\n");
@@ -688,6 +704,15 @@ static int q6v5_wcss_stop(struct rproc *rproc)
 	struct q6v5_wcss *wcss = rproc->priv;
 	int ret;
 
+	if (wcss->need_mem_protection) {
+		ret = qcom_scm_pas_shutdown(WCNSS_PAS_ID);
+		if (ret) {
+			dev_err(wcss->dev, "not able to shutdown\n");
+			return ret;
+		}
+		goto pas_done;
+	}
+
 	/* WCSS powerdown */
 	if (wcss->requires_force_stop) {
 		ret = qcom_q6v5_request_stop(&wcss->q6v5);
@@ -712,6 +737,7 @@ static int q6v5_wcss_stop(struct rproc *rproc)
 			return ret;
 	}
 
+pas_done:
 	clk_disable_unprepare(wcss->prng_clk);
 	qcom_q6v5_unprepare(&wcss->q6v5);
 
@@ -734,6 +760,12 @@ static int q6v5_wcss_load(struct rproc *rproc, const struct firmware *fw)
 {
 	struct q6v5_wcss *wcss = rproc->priv;
 
+	if (wcss->need_mem_protection)
+		return qcom_mdt_load(wcss->dev, fw, rproc->firmware,
+				     WCNSS_PAS_ID, wcss->mem_region,
+				     wcss->mem_phys, wcss->mem_size,
+				     &wcss->mem_reloc);
+
 	return qcom_mdt_load_no_init(wcss->dev, fw, rproc->firmware,
 				     0, wcss->mem_region, wcss->mem_phys,
 				     wcss->mem_size, &wcss->mem_reloc);
@@ -1019,6 +1051,9 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
 	if (!desc)
 		return -EINVAL;
 
+	if (desc->need_mem_protection && !qcom_scm_is_available())
+		return -EPROBE_DEFER;
+
 	rproc = rproc_alloc(&pdev->dev, pdev->name, desc->ops,
 			    desc->firmware_name, sizeof(*wcss));
 	if (!rproc) {
@@ -1032,6 +1067,7 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
 
 	wcss->version = desc->version;
 	wcss->requires_force_stop = desc->requires_force_stop;
+	wcss->need_mem_protection = desc->need_mem_protection;
 
 	ret = q6v5_wcss_init_mmio(wcss, pdev);
 	if (ret)
@@ -1099,6 +1135,7 @@ static int q6v5_wcss_remove(struct platform_device *pdev)
 	.aon_reset_required = true,
 	.ops = &q6v5_wcss_ipq8074_ops,
 	.requires_force_stop = true,
+	.need_mem_protection = true,
 };
 
 static const struct wcss_data wcss_qcs404_res_init = {
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH 01/12] remoteproc: qcom: Add PRNG proxy clock
From: Gokul Sriram Palanisamy @ 2019-07-11 15:40 UTC (permalink / raw)
  To: agross, david.brown, robh+dt, mark.rutland, mturquette, sboyd,
	jassisinghbrar, ohad, bjorn.andersson, linux-arm-msm, devicetree,
	linux-kernel, linux-clk, linux-remoteproc, sricharan, gokulsri
In-Reply-To: <1562859668-14209-1-git-send-email-gokulsri@codeaurora.org>

PRNG clock is needed by the secure PIL, support for the same
is added in subsequent patches.

Signed-off-by: Gokul Sriram Palanisamy <gokulsri@codeaurora.org>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
Signed-off-by: Nikhil Prakash V <nprakash@codeaurora.org>
---
 drivers/remoteproc/qcom_q6v5_wcss.c | 63 +++++++++++++++++++++++++++----------
 1 file changed, 46 insertions(+), 17 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index 0821c94..dc2bae4 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -88,18 +88,6 @@ enum {
 	WCSS_QCS404,
 };
 
-struct wcss_data {
-	const char *firmware_name;
-	int crash_reason_smem;
-	u8 version;
-	bool aon_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 {
 	struct device *dev;
 
@@ -124,6 +112,7 @@ struct q6v5_wcss {
 	struct clk *qdsp6ss_xo_cbcr;
 	struct clk *qdsp6ss_core_gfmux;
 	struct clk *wcss_bcr_cbcr;
+	struct clk *prng_clk;
 	struct regulator *cx_supply;
 
 	struct qcom_rproc_glink glink_subdev;
@@ -146,6 +135,20 @@ struct q6v5_wcss {
 	bool requires_force_stop;
 };
 
+struct wcss_data {
+	int (*init_clock)(struct q6v5_wcss *wcss);
+	int (*init_regulator)(struct q6v5_wcss *wcss);
+	const char *firmware_name;
+	int crash_reason_smem;
+	u8 version;
+	bool aon_reset_required;
+	const char *ssr_name;
+	const char *sysmon_name;
+	int ssctl_id;
+	const struct rproc_ops *ops;
+	bool requires_force_stop;
+};
+
 static int q6v5_wcss_reset(struct q6v5_wcss *wcss)
 {
 	int ret;
@@ -235,6 +238,12 @@ static int q6v5_wcss_start(struct rproc *rproc)
 	struct q6v5_wcss *wcss = rproc->priv;
 	int ret;
 
+	ret = clk_prepare_enable(wcss->prng_clk);
+	if (ret) {
+		dev_err(wcss->dev, "prng clock enable failed\n");
+		return ret;
+	}
+
 	qcom_q6v5_prepare(&wcss->q6v5);
 
 	/* Release Q6 and WCSS reset */
@@ -703,6 +712,7 @@ static int q6v5_wcss_stop(struct rproc *rproc)
 			return ret;
 	}
 
+	clk_disable_unprepare(wcss->prng_clk);
 	qcom_q6v5_unprepare(&wcss->q6v5);
 
 	return 0;
@@ -858,7 +868,21 @@ static int q6v5_alloc_memory_region(struct q6v5_wcss *wcss)
 	return 0;
 }
 
-static int q6v5_wcss_init_clock(struct q6v5_wcss *wcss)
+static int ipq8074_init_clock(struct q6v5_wcss *wcss)
+{
+	int ret;
+
+	wcss->prng_clk = devm_clk_get(wcss->dev, "prng");
+	if (IS_ERR(wcss->prng_clk)) {
+		ret = PTR_ERR(wcss->prng_clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(wcss->dev, "Failed to get prng clock\n");
+		return ret;
+	}
+	return 0;
+}
+
+static int qcs404_init_clock(struct q6v5_wcss *wcss)
 {
 	int ret;
 
@@ -973,7 +997,7 @@ static int q6v5_wcss_init_clock(struct q6v5_wcss *wcss)
 	return 0;
 }
 
-static int q6v5_wcss_init_regulator(struct q6v5_wcss *wcss)
+static int qcs404_init_regulator(struct q6v5_wcss *wcss)
 {
 	wcss->cx_supply = devm_regulator_get(wcss->dev, "cx");
 	if (IS_ERR(wcss->cx_supply))
@@ -1017,12 +1041,14 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_rproc;
 
-	if (wcss->version == WCSS_QCS404) {
-		ret = q6v5_wcss_init_clock(wcss);
+	if (desc->init_clock) {
+		ret = desc->init_clock(wcss);
 		if (ret)
 			goto free_rproc;
+	}
 
-		ret = q6v5_wcss_init_regulator(wcss);
+	if (desc->init_regulator) {
+		ret = desc->init_regulator(wcss);
 		if (ret)
 			goto free_rproc;
 	}
@@ -1067,6 +1093,7 @@ static int q6v5_wcss_remove(struct platform_device *pdev)
 }
 
 static const struct wcss_data wcss_ipq8074_res_init = {
+	.init_clock = ipq8074_init_clock,
 	.firmware_name = "IPQ8074/q6_fw.mdt",
 	.crash_reason_smem = 421,
 	.aon_reset_required = true,
@@ -1075,6 +1102,8 @@ static int q6v5_wcss_remove(struct platform_device *pdev)
 };
 
 static const struct wcss_data wcss_qcs404_res_init = {
+	.init_clock = qcs404_init_clock,
+	.init_regulator = qcs404_init_regulator,
 	.crash_reason_smem = 421,
 	.firmware_name = "wcnss.mdt",
 	.version = WCSS_QCS404,
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox