Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v1 1/5] dt-bindings: pwm: Add Loongson PWM controller
From: Binbin Zhou @ 2024-04-03  2:37 UTC (permalink / raw)
  To: Rob Herring
  Cc: Binbin Zhou, Huacai Chen, Uwe Kleine-König,
	Krzysztof Kozlowski, Conor Dooley, Huacai Chen, loongson-kernel,
	linux-pwm, devicetree, Xuerui Wang, loongarch
In-Reply-To: <20240402174051.GA324804-robh@kernel.org>

Hi Rob:

Thanks for your reply.

On Tue, Apr 2, 2024 at 11:40 PM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Apr 02, 2024 at 03:58:38PM +0800, Binbin Zhou wrote:
> > Add Loongson PWM controller binding with DT schema format using
> > json-schema.
> >
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> >  .../devicetree/bindings/pwm/pwm-loongson.yaml | 64 +++++++++++++++++++
>
> Filename should match compatible.

Emm... How about renaming it as loongson, pwm.yaml?

>
> >  MAINTAINERS                                   |  6 ++
> >  2 files changed, 70 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> > new file mode 100644
> > index 000000000000..d25904468353
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> > @@ -0,0 +1,64 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pwm/pwm-loongson.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Loongson PWM Controller
> > +
> > +maintainers:
> > +  - Binbin Zhou <zhoubinbin@loongson.cn>
> > +
> > +description:
> > +  It is the generic PWM framework driver for Loongson family.
>
> That's describing the driver. Not really relevant to the binding.
>
Ok ,I will rewrite this part.

>
> > +  Each PWM has one pulse width output signal and one pulse input
> > +  signal to be measured.
> > +  It can be found on Loongson-2K series cpus and Loongson LS7A bridge chips.
> > +
> > +allOf:
> > +  - $ref: pwm.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - const: loongson,ls7a-pwm
> > +      - items:
> > +          - enum:
> > +              - loongson,ls2k0500-pwm
> > +              - loongson,ls2k1000-pwm
> > +              - loongson,ls2k2000-pwm
> > +          - const: loongson,ls7a-pwm
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  '#pwm-cells':
> > +    const: 3
>
> Please define what is in each cell. If there's only 2 signals, then the
> first cell defines the output or input (what value for which one?).
>
> Really, the PWM binding is only for outputs, so is a cell even needed? I
> suppose we could use it for inputs too, but that's really "input
> capture" type operation that timers often have. I'll defer to the PWM
> maintainers...

Ok, I will try to add some description about it.

If I understand correctly, the meaning of each cell in "#pwm-cells"is
determined.
The first cell specifies the per-chip index of the PWM to use, the
second cell is the period in nanoseconds and the third cell is the
polarity.

>
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - '#pwm-cells'
>
> pwm.yaml makes this required already.
>
Yes, this is unnecessary. I will drop it in the next version.

Thanks.
Binbin
> Rob
>

^ permalink raw reply

* Re: [PATCH v3 4/4] drm: panel: Add LG sw43408 panel driver
From: Dmitry Baryshkov @ 2024-04-03  2:37 UTC (permalink / raw)
  To: Marijn Suijten
  Cc: Sumit Semwal, Caleb Connolly, Neil Armstrong, Jessica Zhang,
	Sam Ravnborg, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, dri-devel, devicetree,
	linux-kernel, linux-arm-msm, Vinod Koul, Caleb Connolly
In-Reply-To: <3hh7nfqflj73422q47p6do7aiormxwspwkfg557epeqn2krtcc@dsufsohfaz4l>

On Tue, Apr 02, 2024 at 11:17:52PM +0200, Marijn Suijten wrote:
> On 2024-04-02 02:51:15, Dmitry Baryshkov wrote:
> > From: Sumit Semwal <sumit.semwal@linaro.org>
> > 
> > LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel, used in some Pixel3
> > phones.
> 
> @60Hz?

With the current settings and timings I'm only getting 30 Hz. I have to
double the mode->clock to get 60.

> 
> > 
> > Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> > [vinod: Add DSC support]
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > [caleb: cleanup and support turning off the panel]
> > Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> > [DB: partially rewrote the driver and fixed DSC programming]
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> Some small nits but I think this deserves a:
> 
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> 
> > ---
> >  MAINTAINERS                              |   8 +
> >  drivers/gpu/drm/panel/Kconfig            |  11 ++
> >  drivers/gpu/drm/panel/Makefile           |   1 +
> >  drivers/gpu/drm/panel/panel-lg-sw43408.c | 326 +++++++++++++++++++++++++++++++
> >  4 files changed, 346 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index d36c19c1bf81..4cc43c16e07e 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6789,6 +6789,14 @@ S:	Maintained
> >  F:	Documentation/devicetree/bindings/display/panel/jadard,jd9365da-h3.yaml
> >  F:	drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> >  
> > +DRM DRIVER FOR LG SW43408 PANELS
> > +M:	Sumit Semwal <sumit.semwal@linaro.org>
> > +M:	Caleb Connolly <caleb.connolly@linaro.org>
> > +S:	Maintained
> > +T:	git git://anongit.freedesktop.org/drm/drm-misc
> > +F:	Documentation/devicetree/bindings/display/panel/lg,sw43408.yaml
> > +F:	drivers/gpu/drm/panel/panel-lg-sw43408.c
> > +
> >  DRM DRIVER FOR LOGICVC DISPLAY CONTROLLER
> >  M:	Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> >  S:	Supported
> > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > index 6dc451f58a3e..a55e9437c8cf 100644
> > --- a/drivers/gpu/drm/panel/Kconfig
> > +++ b/drivers/gpu/drm/panel/Kconfig
> > @@ -335,6 +335,17 @@ config DRM_PANEL_LG_LG4573
> >  	  Say Y here if you want to enable support for LG4573 RGB panel.
> >  	  To compile this driver as a module, choose M here.
> >  
> > +config DRM_PANEL_LG_SW43408
> > +	tristate "LG SW43408 panel"
> > +	depends on OF
> > +	depends on DRM_MIPI_DSI
> > +	depends on BACKLIGHT_CLASS_DEVICE
> > +	help
> > +	  Say Y here if you want to enable support for LG sw43408 panel.
> > +	  The panel has a 1080x2160 resolution and uses
> > +	  24 bit RGB per pixel. It provides a MIPI DSI interface to
> > +	  the host and has a built-in LED backlight.
> > +
> >  config DRM_PANEL_MAGNACHIP_D53E6EA8966
> >  	tristate "Magnachip D53E6EA8966 DSI panel"
> >  	depends on OF && SPI
> > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > index 24a02655d726..0b40b010e8e7 100644
> > --- a/drivers/gpu/drm/panel/Makefile
> > +++ b/drivers/gpu/drm/panel/Makefile
> > @@ -34,6 +34,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
> >  obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
> >  obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
> >  obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> > +obj-$(CONFIG_DRM_PANEL_LG_SW43408) += panel-lg-sw43408.o
> >  obj-$(CONFIG_DRM_PANEL_MAGNACHIP_D53E6EA8966) += panel-magnachip-d53e6ea8966.o
> >  obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
> >  obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3051D) += panel-newvision-nv3051d.o
> > diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> > new file mode 100644
> > index 000000000000..c7611bfa796b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> > @@ -0,0 +1,326 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2019-2024 Linaro Ltd
> > + * Author: Sumit Semwal <sumit.semwal@linaro.org>
> > + *	 Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > + */
> > +
> > +#include <linux/backlight.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/regulator/consumer.h>
> > +
> > +#include <video/mipi_display.h>
> > +
> > +#include <drm/drm_mipi_dsi.h>
> > +#include <drm/drm_panel.h>
> > +#include <drm/drm_probe_helper.h>
> > +#include <drm/display/drm_dsc.h>
> > +#include <drm/display/drm_dsc_helper.h>
> > +
> > +#define NUM_SUPPLIES 2
> > +
> > +struct sw43408_panel {
> > +	struct drm_panel base;
> > +	struct mipi_dsi_device *link;
> > +
> > +	const struct drm_display_mode *mode;
> > +
> > +	struct regulator_bulk_data supplies[NUM_SUPPLIES];
> > +
> > +	struct gpio_desc *reset_gpio;
> > +
> > +	struct drm_dsc_config dsc;
> > +};
> > +
> > +static inline struct sw43408_panel *to_panel_info(struct drm_panel *panel)
> > +{
> > +	return container_of(panel, struct sw43408_panel, base);
> > +}
> > +
> > +static int sw43408_unprepare(struct drm_panel *panel)
> > +{
> > +	struct sw43408_panel *ctx = to_panel_info(panel);
> > +	int ret;
> > +
> > +	ret = mipi_dsi_dcs_set_display_off(ctx->link);
> > +	if (ret < 0)
> > +		dev_err(panel->dev, "set_display_off cmd failed ret = %d\n", ret);
> > +
> > +	ret = mipi_dsi_dcs_enter_sleep_mode(ctx->link);
> > +	if (ret < 0)
> > +		dev_err(panel->dev, "enter_sleep cmd failed ret = %d\n", ret);
> > +
> > +	msleep(100);
> > +
> > +	gpiod_set_value(ctx->reset_gpio, 1);
> > +
> > +	return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > +}
> > +
> > +static int sw43408_program(struct drm_panel *panel)
> > +{
> > +	struct sw43408_panel *ctx = to_panel_info(panel);
> > +	struct drm_dsc_picture_parameter_set pps;
> > +
> > +	mipi_dsi_dcs_write_seq(ctx->link, MIPI_DCS_SET_GAMMA_CURVE, 0x02);
> > +
> > +	mipi_dsi_dcs_set_tear_on(ctx->link, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> > +
> > +	mipi_dsi_dcs_write_seq(ctx->link, 0x53, 0x0c, 0x30);
> > +	mipi_dsi_dcs_write_seq(ctx->link, 0x55, 0x00, 0x70, 0xdf, 0x00, 0x70, 0xdf);
> > +	mipi_dsi_dcs_write_seq(ctx->link, 0xf7, 0x01, 0x49, 0x0c);
> > +
> > +	mipi_dsi_dcs_exit_sleep_mode(ctx->link);
> > +
> > +	msleep(135);
> > +
> > +	/* COMPRESSION_MODE moved after setting the PPS */
> > +
> > +	mipi_dsi_dcs_write_seq(ctx->link, 0xb0, 0xac);
> > +	mipi_dsi_dcs_write_seq(ctx->link, 0xe5,
> > +			       0x00, 0x3a, 0x00, 0x3a, 0x00, 0x0e, 0x10);
> > +	mipi_dsi_dcs_write_seq(ctx->link, 0xb5,
> > +			       0x75, 0x60, 0x2d, 0x5d, 0x80, 0x00, 0x0a, 0x0b,
> > +			       0x00, 0x05, 0x0b, 0x00, 0x80, 0x0d, 0x0e, 0x40,
> > +			       0x00, 0x0c, 0x00, 0x16, 0x00, 0xb8, 0x00, 0x80,
> > +			       0x0d, 0x0e, 0x40, 0x00, 0x0c, 0x00, 0x16, 0x00,
> > +			       0xb8, 0x00, 0x81, 0x00, 0x03, 0x03, 0x03, 0x01,
> > +			       0x01);
> > +	msleep(85);
> > +	mipi_dsi_dcs_write_seq(ctx->link, 0xcd,
> > +			       0x00, 0x00, 0x00, 0x19, 0x19, 0x19, 0x19, 0x19,
> > +			       0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x19,
> > +			       0x16, 0x16);
> > +	mipi_dsi_dcs_write_seq(ctx->link, 0xcb, 0x80, 0x5c, 0x07, 0x03, 0x28);
> > +	mipi_dsi_dcs_write_seq(ctx->link, 0xc0, 0x02, 0x02, 0x0f);
> > +	mipi_dsi_dcs_write_seq(ctx->link, 0x55, 0x04, 0x61, 0xdb, 0x04, 0x70, 0xdb);
> > +	mipi_dsi_dcs_write_seq(ctx->link, 0xb0, 0xca);
> > +
> > +	mipi_dsi_dcs_set_display_on(ctx->link);
> > +
> > +	msleep(50);
> > +
> > +	ctx->link->mode_flags &= ~MIPI_DSI_MODE_LPM;
> > +
> > +	drm_dsc_pps_payload_pack(&pps, ctx->link->dsc);
> > +	mipi_dsi_picture_parameter_set(ctx->link, &pps);
> > +
> > +	/* This panel uses shifted PPS selectors:
> > +	 * 1 if pps_identifier is 0
> > +	 * 2 if pps_identifier is 1
> > +	 */
> > +	mipi_dsi_compression_mode_ext(ctx->link, true,
> > +				      MIPI_DSI_COMPRESSION_DSC, 1);
> 
> Let's be careful to watch the order of parameters here whichever way you fix it
> up in the patch that introduces this function.
> 
> > +
> > +	ctx->link->mode_flags |= MIPI_DSI_MODE_LPM;
> > +
> > +	return 0;
> > +}
> > +
> > +static int sw43408_prepare(struct drm_panel *panel)
> > +{
> > +	struct sw43408_panel *ctx = to_panel_info(panel);
> > +	int ret;
> > +
> > +	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	usleep_range(5000, 6000);
> > +
> > +	gpiod_set_value(ctx->reset_gpio, 0);
> > +	usleep_range(9000, 10000);
> > +	gpiod_set_value(ctx->reset_gpio, 1);
> > +	usleep_range(1000, 2000);
> > +	gpiod_set_value(ctx->reset_gpio, 0);
> > +	usleep_range(9000, 10000);
> > +
> > +	ret = sw43408_program(panel);
> > +	if (ret)
> > +		goto poweroff;
> > +
> > +	return 0;
> > +
> > +poweroff:
> > +	gpiod_set_value(ctx->reset_gpio, 1);
> > +	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > +	return ret;
> > +}
> > +
> > +static int sw43408_get_modes(struct drm_panel *panel,
> > +			      struct drm_connector *connector)
> > +{
> > +	struct sw43408_panel *ctx = to_panel_info(panel);
> > +
> > +	return drm_connector_helper_get_modes_fixed(connector, ctx->mode);
> > +}
> > +
> > +static int sw43408_backlight_update_status(struct backlight_device *bl)
> > +{
> > +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> > +	uint16_t brightness = backlight_get_brightness(bl);
> > +
> > +	return mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
> > +}
> > +
> > +const struct backlight_ops sw43408_backlight_ops = {
> > +	.update_status = sw43408_backlight_update_status,
> > +};
> > +
> > +static int sw43408_backlight_init(struct sw43408_panel *ctx)
> > +{
> > +	struct device *dev = &ctx->link->dev;
> > +	const struct backlight_properties props = {
> > +		.type = BACKLIGHT_PLATFORM,
> > +		.brightness = 255,
> > +		.max_brightness = 255,
> > +	};
> > +
> > +	ctx->base.backlight = devm_backlight_device_register(dev, dev_name(dev), dev,
> > +							ctx->link,
> > +							&sw43408_backlight_ops,
> > +							&props);
> > +
> > +	if (IS_ERR(ctx->base.backlight))
> > +		return dev_err_probe(dev, PTR_ERR(ctx->base.backlight),
> > +				     "Failed to create backlight\n");
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct drm_panel_funcs sw43408_funcs = {
> > +	.unprepare = sw43408_unprepare,
> > +	.prepare = sw43408_prepare,
> > +	.get_modes = sw43408_get_modes,
> > +};
> > +
> > +static const struct drm_display_mode sw43408_default_mode = {
> > +	.clock = 152340,
> 
> Since this value is calculated from the values below, I prefer to just show the
> origin of the value:
> 
> 	.clock = (1080 + 20 + 32 + 20) * (2160 + 20 + 4 + 20) * 60 / 1000,

Sure

> 
> > +
> > +	.hdisplay = 1080,
> > +	.hsync_start = 1080 + 20,
> > +	.hsync_end = 1080 + 20 + 32,
> > +	.htotal = 1080 + 20 + 32 + 20,
> > +
> > +	.vdisplay = 2160,
> > +	.vsync_start = 2160 + 20,
> > +	.vsync_end = 2160 + 20 + 4,
> > +	.vtotal = 2160 + 20 + 4 + 20,
> > +
> > +	.width_mm = 62,
> > +	.height_mm = 124,
> > +
> > +	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> > +};
> > +
> > +static const struct of_device_id sw43408_of_match[] = {
> > +	{ .compatible = "lg,sw43408", .data = &sw43408_default_mode },
> 
> Will you ever use multiple compatibles to select different modes?
> 
> For panels that support multiple modes (e.g. a lot of high-end sony devices
> with their 4k@120Hz screens) I'm still planning on adding an atomic_prepare() to
> drm_bridge and drm_panel to make it possible to program the DSC block and send
> DCS relative to the selected mode (and/or perform a fluent mode switch).

I think this got inherited from the initial implementation by Sumit.
Let's rewrite this too.

> 
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, sw43408_of_match);
> > +
> > +static int sw43408_add(struct sw43408_panel *ctx)
> > +{
> > +	struct device *dev = &ctx->link->dev;
> > +	int ret;
> > +
> > +	ctx->supplies[0].supply = "vddi"; /* 1.88 V */
> > +	ctx->supplies[0].init_load_uA = 62000;
> > +	ctx->supplies[1].supply = "vpnl"; /* 3.0 V */
> > +	ctx->supplies[1].init_load_uA = 857000;
> > +
> > +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
> > +				      ctx->supplies);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > +	if (IS_ERR(ctx->reset_gpio)) {
> > +		dev_err(dev, "cannot get reset gpio %ld\n",
> > +			      PTR_ERR(ctx->reset_gpio));
> > +		return PTR_ERR(ctx->reset_gpio);
> > +	}
> > +
> > +	ret = sw43408_backlight_init(ctx);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ctx->base.prepare_prev_first = true;
> > +
> > +	drm_panel_init(&ctx->base, dev, &sw43408_funcs, DRM_MODE_CONNECTOR_DSI);
> > +
> > +	drm_panel_add(&ctx->base);
> > +	return ret;
> > +}
> > +
> > +static int sw43408_probe(struct mipi_dsi_device *dsi)
> > +{
> > +	struct sw43408_panel *ctx;
> > +	int ret;
> > +
> > +	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
> > +	if (!ctx)
> > +		return -ENOMEM;
> > +
> > +	ctx->mode = of_device_get_match_data(&dsi->dev);
> > +	dsi->mode_flags = MIPI_DSI_MODE_LPM;
> > +	dsi->format = MIPI_DSI_FMT_RGB888;
> > +	dsi->lanes = 4;
> > +
> > +	ctx->link = dsi;
> > +	mipi_dsi_set_drvdata(dsi, ctx);
> > +
> > +	ret = sw43408_add(ctx);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* The panel is DSC panel only, set the dsc params */
> 
> Grammar?
> 
> > +	ctx->dsc.dsc_version_major = 0x1;
> > +	ctx->dsc.dsc_version_minor = 0x1;
> > +
> > +	/* slice_count * slice_width == width */
> > +	ctx->dsc.slice_height = 16;
> > +	ctx->dsc.slice_width = 540;
> > +	ctx->dsc.slice_count = 2;
> > +	ctx->dsc.bits_per_component = 8;
> > +	ctx->dsc.bits_per_pixel = 8 << 4;
> > +	ctx->dsc.block_pred_enable = true;
> > +
> > +	dsi->dsc = &ctx->dsc;
> > +
> > +	return mipi_dsi_attach(dsi);
> > +}
> > +
> > +static void sw43408_remove(struct mipi_dsi_device *dsi)
> > +{
> > +	struct sw43408_panel *ctx = mipi_dsi_get_drvdata(dsi);
> > +	int ret;
> > +
> > +	ret = sw43408_unprepare(&ctx->base);
> > +	if (ret < 0)
> > +		dev_err(&dsi->dev, "failed to unprepare panel: %d\n",
> > +			      ret);
> > +
> > +	ret = mipi_dsi_detach(dsi);
> > +	if (ret < 0)
> > +		dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", ret);
> > +
> > +	drm_panel_remove(&ctx->base);
> > +}
> > +
> > +static struct mipi_dsi_driver sw43408_driver = {
> > +	.driver = {
> > +		.name = "panel-lg-sw43408",
> > +		.of_match_table = sw43408_of_match,
> > +	},
> > +	.probe = sw43408_probe,
> > +	.remove = sw43408_remove,
> > +};
> > +module_mipi_dsi_driver(sw43408_driver);
> > +
> > +MODULE_AUTHOR("Sumit Semwal <sumit.semwal@linaro.org>");
> > +MODULE_DESCRIPTION("LG SW436408 MIPI-DSI LED panel");
> > +MODULE_LICENSE("GPL");
> > 
> > -- 
> > 2.39.2
> > 

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH v3 3/4] drm/mipi-dsi: add mipi_dsi_compression_mode_ext()
From: Dmitry Baryshkov @ 2024-04-03  2:35 UTC (permalink / raw)
  To: Marijn Suijten
  Cc: Sumit Semwal, Caleb Connolly, Neil Armstrong, Jessica Zhang,
	Sam Ravnborg, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, dri-devel, devicetree,
	linux-kernel, linux-arm-msm
In-Reply-To: <dgvztw55cr4wsbfxhf4ondzhaetodfcv4pndfug73suae5vp3v@4ozlmmsemfbm>

On Tue, Apr 02, 2024 at 11:09:29PM +0200, Marijn Suijten wrote:
> On 2024-04-02 02:51:14, Dmitry Baryshkov wrote:
> > Add the extended version of mipi_dsi_compression_mode(). It provides
> > a way to specify the algorithm and PPS selector.
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  drivers/gpu/drm/drm_mipi_dsi.c | 33 +++++++++++++++++++++++++++------
> >  include/drm/drm_mipi_dsi.h     |  9 +++++++++
> >  2 files changed, 36 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> > index 9874ff6d4718..0ecbc811eb7a 100644
> > --- a/drivers/gpu/drm/drm_mipi_dsi.c
> > +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> > @@ -645,19 +645,24 @@ int mipi_dsi_set_maximum_return_packet_size(struct mipi_dsi_device *dsi,
> >  EXPORT_SYMBOL(mipi_dsi_set_maximum_return_packet_size);
> >  
> >  /**
> > - * mipi_dsi_compression_mode() - enable/disable DSC on the peripheral
> > + * mipi_dsi_compression_mode_ext() - enable/disable DSC on the peripheral
> >   * @dsi: DSI peripheral device
> >   * @enable: Whether to enable or disable the DSC
> > + * @algo: Selected algorithm
> > + * @pps_selector: The PPS selector
> 
> Not a big fan of paraphrasing the parameter name, it adds no value.  How about
> describing what this parameter means and what it does?:
> 
> 	PPS table index to use.  Corresponds to a table pre-programmed on the peripheral
> 	or a table programmed with &drm_dsc_picture_parameter_set.pps_identifier.
> 
> (That should be a valid kernel-doc cross-reference to the field)

I don't think such meaning is defined in the standard. In the end, the
pps_identifier is 8-bit wide, while this field has just 2 bits and 4
possible values. The standards are pretty vague about this field. 

> 
> >   *
> > - * Enable or disable Display Stream Compression on the peripheral using the
> > - * default Picture Parameter Set and VESA DSC 1.1 algorithm.
> > + * Enable or disable Display Stream Compression on the peripheral.
> >   *
> >   * Return: 0 on success or a negative error code on failure.
> >   */
> > -int mipi_dsi_compression_mode(struct mipi_dsi_device *dsi, bool enable)
> > +int mipi_dsi_compression_mode_ext(struct mipi_dsi_device *dsi, bool enable,
> > +				  enum mipi_dsi_compression_algo algo,
> > +				  unsigned int pps_selector)
> >  {
> > -	/* Note: Needs updating for non-default PPS or algorithm */
> > -	u8 tx[2] = { enable << 0, 0 };
> > +	u8 data = (enable << 0) |
> > +		(algo << 1) |
> > +		(pps_selector << 4);
> 
> Do we need some size validation (if > 3 return -EINVAL)?  FIELD_PREP() might be
> too heavy though.

Ack, let's add it.

> 
> > +	u8 tx[2] = { data, 0 };
> >  	struct mipi_dsi_msg msg = {
> >  		.channel = dsi->channel,
> >  		.type = MIPI_DSI_COMPRESSION_MODE,
> > @@ -668,6 +673,22 @@ int mipi_dsi_compression_mode(struct mipi_dsi_device *dsi, bool enable)
> >  
> >  	return (ret < 0) ? ret : 0;
> >  }
> > +EXPORT_SYMBOL(mipi_dsi_compression_mode_ext);
> > +
> > +/**
> > + * mipi_dsi_compression_mode() - enable/disable DSC on the peripheral
> > + * @dsi: DSI peripheral device
> > + * @enable: Whether to enable or disable the DSC
> > + *
> > + * Enable or disable Display Stream Compression on the peripheral using the
> > + * default Picture Parameter Set and VESA DSC 1.1 algorithm.
> > + *
> > + * Return: 0 on success or a negative error code on failure.
> > + */
> > +int mipi_dsi_compression_mode(struct mipi_dsi_device *dsi, bool enable)
> > +{
> > +	return mipi_dsi_compression_mode_ext(dsi, enable, 0, MIPI_DSI_COMPRESSION_DSC);
> 
> I hope the compiler complains here that it should be MIPI_DSI_COMPRESSION_DSC,0

Nope, it didn't. Both are integers.

> 
> (Enum algo first, int pps_selector last)
> 
> > +}
> >  EXPORT_SYMBOL(mipi_dsi_compression_mode);
> >  
> >  /**
> > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> > index 3011d33eccbd..78cb7b688b1d 100644
> > --- a/include/drm/drm_mipi_dsi.h
> > +++ b/include/drm/drm_mipi_dsi.h
> > @@ -226,6 +226,12 @@ static inline int mipi_dsi_pixel_format_to_bpp(enum mipi_dsi_pixel_format fmt)
> >  	return -EINVAL;
> >  }
> >  
> > +enum mipi_dsi_compression_algo {
> > +	MIPI_DSI_COMPRESSION_DSC = 0,
> 
> Add 1.1?  Or does it also allow 1.2 (when the version is also set via PPS)?

I have only DSI 1.3 at hand, which only talks about 1.1. I think 1.2 is
allowed by inheritance. That's why I skipped the version here.

> 
> > +	MIPI_DSI_COMPRESSION_VENDOR = 3,
> > +	/* other two values are reserved, DSI 1.3 */
> > +};
> > +
> >  struct mipi_dsi_device *
> >  mipi_dsi_device_register_full(struct mipi_dsi_host *host,
> >  			      const struct mipi_dsi_device_info *info);
> > @@ -242,6 +248,9 @@ int mipi_dsi_turn_on_peripheral(struct mipi_dsi_device *dsi);
> >  int mipi_dsi_set_maximum_return_packet_size(struct mipi_dsi_device *dsi,
> >  					    u16 value);
> >  int mipi_dsi_compression_mode(struct mipi_dsi_device *dsi, bool enable);
> > +int mipi_dsi_compression_mode_ext(struct mipi_dsi_device *dsi, bool enable,
> > +				  unsigned int pps_selector,
> > +				  enum mipi_dsi_compression_algo algo);
> 
> Oh, this declaration is inverse from the definition...

I'll check it.

> 
> - Marijn
> 
> >  int mipi_dsi_picture_parameter_set(struct mipi_dsi_device *dsi,
> >  				   const struct drm_dsc_picture_parameter_set *pps);
> >  
> > 
> > -- 
> > 2.39.2
> > 

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH v7 2/2] dmaengine: Loongson1: Add Loongson-1 APB DMA driver
From: Keguang Zhang @ 2024-04-03  2:23 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Vinod Koul, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-mips, dmaengine, devicetree, linux-kernel
In-Reply-To: <CAAhV-H5BSS1k6aCLYg4DT12nq-d4YyAKYBvy2ucLydwjVKNhBw@mail.gmail.com>

On Tue, Apr 2, 2024 at 10:50 PM Huacai Chen <chenhuacai@kernel.org> wrote:
>
> On Tue, Apr 2, 2024 at 6:51 PM Keguang Zhang <keguang.zhang@gmail.com> wrote:
> >
> > On Tue, Apr 2, 2024 at 5:04 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > >
> > > On Tue, Apr 2, 2024 at 9:56 AM Keguang Zhang <keguang.zhang@gmail.com> wrote:
> > > >
> > > > On Mon, Apr 1, 2024 at 9:24 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > >
> > > > > On Mon, Apr 1, 2024 at 7:10 PM Keguang Zhang <keguang.zhang@gmail.com> wrote:
> > > > > >
> > > > > > On Mon, Apr 1, 2024 at 5:06 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > > > >
> > > > > > > On Mon, Apr 1, 2024 at 10:45 AM Keguang Zhang <keguang.zhang@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Hi Huacai,
> > > > > > > >
> > > > > > > > On Sat, Mar 30, 2024 at 9:59 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > > > > > >
> > > > > > > > > Hi, Keguang,
> > > > > > > > >
> > > > > > > > > On Fri, Mar 29, 2024 at 7:28 PM Keguang Zhang via B4 Relay
> > > > > > > > > <devnull+keguang.zhang.gmail.com@kernel.org> wrote:
> > > > > > > > > >
> > > > > > > > > > From: Keguang Zhang <keguang.zhang@gmail.com>
> > > > > > > > > >
> > > > > > > > > > This patch adds APB DMA driver for Loongson-1 SoCs.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
> > > > > > > > > > ---
> > > > > > > > > > Changes in v7:
> > > > > > > > > > - Change the comptible to 'loongson,ls1*-apbdma'
> > > > > > > > > > - Update Kconfig and Makefile accordingly
> > > > > > > > > > - Rename the file to loongson1-apb-dma.c to keep the consistency
> > > > > > > > > >
> > > > > > > > > > Changes in v6:
> > > > > > > > > > - Implement .device_prep_dma_cyclic for Loongson1 audio driver,
> > > > > > > > > > - as well as .device_pause and .device_resume.
> > > > > > > > > > - Set the limitation LS1X_DMA_MAX_DESC and put all descriptors
> > > > > > > > > > - into one page to save memory
> > > > > > > > > > - Move dma_pool_zalloc() into ls1x_dma_alloc_desc()
> > > > > > > > > > - Drop dma_slave_config structure
> > > > > > > > > > - Use .remove_new instead of .remove
> > > > > > > > > > - Use KBUILD_MODNAME for the driver name
> > > > > > > > > > - Improve the debug information
> > > > > > > > > >
> > > > > > > > > > Changes in v5:
> > > > > > > > > > - Add DT support
> > > > > > > > > > - Use DT data instead of platform data
> > > > > > > > > > - Use chan_id of struct dma_chan instead of own id
> > > > > > > > > > - Use of_dma_xlate_by_chan_id() instead of ls1x_dma_filter()
> > > > > > > > > > - Update the author information to my official name
> > > > > > > > > >
> > > > > > > > > > Changes in v4:
> > > > > > > > > > - Use dma_slave_map to find the proper channel.
> > > > > > > > > > - Explicitly call devm_request_irq() and tasklet_kill().
> > > > > > > > > > - Fix namespace issue.
> > > > > > > > > > - Some minor fixes and cleanups.
> > > > > > > > > >
> > > > > > > > > > Changes in v3:
> > > > > > > > > > - Rename ls1x_dma_filter_fn to ls1x_dma_filter.
> > > > > > > > > >
> > > > > > > > > > Changes in v2:
> > > > > > > > > > - Change the config from 'DMA_LOONGSON1' to 'LOONGSON1_DMA',
> > > > > > > > > > - and rearrange it in alphabetical order in Kconfig and Makefile.
> > > > > > > > > > - Fix comment style.
> > > > > > > > > > ---
> > > > > > > > > >  drivers/dma/Kconfig             |   9 +
> > > > > > > > > >  drivers/dma/Makefile            |   1 +
> > > > > > > > > >  drivers/dma/loongson1-apb-dma.c | 665 ++++++++++++++++++++++++++++++++++++++++
> > > > > > > > > >  3 files changed, 675 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> > > > > > > > > > index 002a5ec80620..f7b06c4cdf3f 100644
> > > > > > > > > > --- a/drivers/dma/Kconfig
> > > > > > > > > > +++ b/drivers/dma/Kconfig
> > > > > > > > > > @@ -369,6 +369,15 @@ config K3_DMA
> > > > > > > > > >           Support the DMA engine for Hisilicon K3 platform
> > > > > > > > > >           devices.
> > > > > > > > > >
> > > > > > > > > > +config LOONGSON1_APB_DMA
> > > > > > > > > > +       tristate "Loongson1 APB DMA support"
> > > > > > > > > > +       depends on MACH_LOONGSON32 || COMPILE_TEST
> > > > > > > > > > +       select DMA_ENGINE
> > > > > > > > > > +       select DMA_VIRTUAL_CHANNELS
> > > > > > > > > > +       help
> > > > > > > > > > +         This selects support for the APB DMA controller in Loongson1 SoCs,
> > > > > > > > > > +         which is required by Loongson1 NAND and audio support.
> > > > > > > > > Why not rename to LS1X_APB_DMA and put it just before LS2X_APB_DMA
> > > > > > > > > (and also the driver file name)?
> > > > > > > > >
> > > > > > > > So far all Kconfig entries of Loongson-1 drivers are named with the
> > > > > > > > keyword "LOONGSON1".
> > > > > > > > The same is true for these file names.
> > > > > > > > Therefore, I need to keep the consistency.
> > > > > > > But I see LS1X_IRQ in drivers/irqchip/Kconfig
> > > > > > >
> > > > > > Indeed, that's an exception, which was submitted by Jiaxun several years ago.
> > > > > > Actually, most drivers of Loongson family use the keyword "LOONGSON"
> > > > > > for Kconfig and "loongson" for filename.
> > > > > > Thus I take this keywork as the naming convention.
> > > > > But I think keeping consistency in a same subsystem is better than
> > > > > keeping consistency in a same SoC (but cross subsystems).
> > > > >
> > > > In my opinion, "LS*X" is too short and may be confused with other SoCs.
> > > > Meanwhile, there are only four drivers that use this keyword.
> > > >   config I2C_LS2X
> > > >   config LS2K_RESET
> > > >   config LS2X_APB_DMA
> > > >   config LS1X_IRQ
> > > > Then, my suggestion is to change these "LS*X" to "LOONGSON*" to get a
> > > > clear meaning.
> > > We have made a naming conversion some years before with Jiaxun.
> > > 1, Use "Loongson" for CPU in arch code;
> > > 2, Use "LS7A" or something like this for bridges and devices.
> > > 3, For drivers in SoC, if the driver is specific to Loongson-1, use
> > > LS1X, if it is to Loongson-2, use LS2X, if it is shared by both
> > > Loongson-1 and Loongson-2, use LOONGSON.
> > >
> > OK. But the doesn't the answer the question of confusion, such as
> > "Freescale LS1021A".
> > The same problem happens to the filenames.
> >   ./drivers/gpu/drm/nouveau/nvkm/nvfw/ls.c
> >   ./drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
> >   ./drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> >   ./drivers/gpu/drm/amd/amdgpu/lsdma_v7_0.c
> >   ./arch/powerpc/platforms/embedded6xx/ls_uart.c
> > Regarding "LS*X" itself, it contains the wildcard character "X" which
> > itself is confusing.
> > Therefore, I don't think "LS*X" is clear enough.
> >
The confusion problem remains.
Honestly, I don't think "LS" is a good short for "LOONGSON".

> > On the other hand, I see "LOONGSON2_*" strings are still there.
This question remains.

> > In addition, some of "LOONGSON_" definitions are not applicable for
> > Loongson-1 at all, which breaks your convention.
> >   config SND_SOC_LOONGSON_I2S_PCI  /* Loongson-1 doesn't support I2S */
> >   config SND_SOC_LOONGSON_CARD
> They are shared by LS2K and LS7A.
>
> >   config DWMAC_LOONGSON1
> >   config DWMAC_LOONGSON  /* This glue layer doesn't support Loongson-1 */
> >   config COMMON_CLK_LOONGSON2
unaddressed

> >   config RTC_DRV_LOONGSON
> RTC is shared by LS2K and LS7A.
>
> >   config SPI_LOONGSON_CORE
> >   config SPI_LOONGSON_PCI  /* N/A for Loongson-1 */
> >   config SPI_LOONGSON_PLATFORM
> SPI is also shared by LS2K and LS7A.
>
> >   config LOONGSON2_CPUFREQ
unaddressed

> >   config DRM_LOONGSON  /* N/A for Loongson-1 */
> DRM is also shared by LS2K and LS7A.
>
> >   config LOONGSON1_WDT
> >   config CLKSRC_LOONGSON1_PWM
> >   config LOONGSON_LIOINTC  /* N/A for Loongson-1 */
> >   config LOONGSON_EIOINTC  /* N/A for Loongson-1 */
> >   config LOONGSON_HTPIC  /* N/A for Loongson-1 */
> >   config LOONGSON_HTVEC  /* N/A for Loongson-1 */
> >   config LOONGSON_PCH_PIC  /* N/A for Loongson-1 */
> >   config LOONGSON_PCH_MSI  /* N/A for Loongson-1 */
> >   config LOONGSON_PCH_LPC  /* N/A for Loongson-1 */
> All interrupt controllers are shared by Loongson-2 and Loongson-3.
>
> >   config PINCTRL_LOONGSON2
unaddressed
> >   config LOONGSON2_THERMAL
ditto
> >   config LOONGSON2_GUTS
ditto
> >   config LOONGSON2_PM
ditto
> >   config LOONGSON_LAPTOP  /* N/A for Loongson-1 */
> Laptop driver is shared by Loongson-2 and Loongson-3.
>
> >   config GPIO_LOONGSON
> >   config GPIO_LOONGSON_64BIT  -> N/A for Loongson-1
> >   config GPIO_LOONGSON1
> GPIO driver is shared by LS2K and LS7A.
>
> >   config PCI_LOONGSON
> PCI driver is shared by Loongson-2 and Loongson-3.

You said "if it is shared by both Loongson-1 and Loongson-2, use LOONGSON."
Now the rule changes from "Loongson-1 and Loongson-2" to "Loongson-2
and Loongson-3".
Then, when shall we use "LOONGSON"?

Here is the situation: only are four drivers use "LS*".
  config I2C_LS2X
  config LS2K_RESET
  config LS2X_APB_DMA
  config LS1X_IRQ
My suggestion is to use the intuitive "LOONGSON*" for both CPU and
drivers, which is easy to understand.
And replace the confusing and unclear "LS*X" with "LOONGSON*".
Use "LOONGSON" when the driver/feature is shared with Loongson-1,
Loongson-2 and Loongson-3.
>
> >
> > What's your plan about the above Kconfig entries?
> Yes, there are exceptions indeed, but very rare. And some of the
> exceptions are due to the limited spare time of Jiaxun and me. But in
> this case, it is better to keep consistency in the DMA subsystem.
>
Sorry, I'm not persuaded.
Please consider my proposal.
Thanks!

> Huacai
>
> > Why can't we use LOONGSON1/LOONGSON2 for drivers?
> >
> >
> > > Huacai
> > >
> > > >
> > > > > Huacai
> > > > >
> > > > > >
> > > > > > > Huacai
> > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > > Huacai
> > > > > > > > >
> > > > > > > > > > +
> > > > > > > > > >  config LPC18XX_DMAMUX
> > > > > > > > > >         bool "NXP LPC18xx/43xx DMA MUX for PL080"
> > > > > > > > > >         depends on ARCH_LPC18XX || COMPILE_TEST
> > > > > > > > > > diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> > > > > > > > > > index dfd40d14e408..b26f6677978a 100644
> > > > > > > > > > --- a/drivers/dma/Makefile
> > > > > > > > > > +++ b/drivers/dma/Makefile
> > > > > > > > > > @@ -47,6 +47,7 @@ obj-$(CONFIG_INTEL_IDMA64) += idma64.o
> > > > > > > > > >  obj-$(CONFIG_INTEL_IOATDMA) += ioat/
> > > > > > > > > >  obj-y += idxd/
> > > > > > > > > >  obj-$(CONFIG_K3_DMA) += k3dma.o
> > > > > > > > > > +obj-$(CONFIG_LOONGSON1_APB_DMA) += loongson1-apb-dma.o
> > > > > > > > > >  obj-$(CONFIG_LPC18XX_DMAMUX) += lpc18xx-dmamux.o
> > > > > > > > > >  obj-$(CONFIG_LS2X_APB_DMA) += ls2x-apb-dma.o
> > > > > > > > > >  obj-$(CONFIG_MILBEAUT_HDMAC) += milbeaut-hdmac.o
> > > > > > > > > > diff --git a/drivers/dma/loongson1-apb-dma.c b/drivers/dma/loongson1-apb-dma.c
> > > > > > > > > > new file mode 100644
> > > > > > > > > > index 000000000000..d474a2601e6e
> > > > > > > > > > --- /dev/null
> > > > > > > > > > +++ b/drivers/dma/loongson1-apb-dma.c
> > > > > > > > > > @@ -0,0 +1,665 @@
> > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > > > > > > > +/*
> > > > > > > > > > + * Driver for Loongson-1 APB DMA Controller
> > > > > > > > > > + *
> > > > > > > > > > + * Copyright (C) 2015-2024 Keguang Zhang <keguang.zhang@gmail.com>
> > > > > > > > > > + */
> > > > > > > > > > +
> > > > > > > > > > +#include <linux/dmapool.h>
> > > > > > > > > > +#include <linux/dma-mapping.h>
> > > > > > > > > > +#include <linux/init.h>
> > > > > > > > > > +#include <linux/interrupt.h>
> > > > > > > > > > +#include <linux/iopoll.h>
> > > > > > > > > > +#include <linux/module.h>
> > > > > > > > > > +#include <linux/of.h>
> > > > > > > > > > +#include <linux/of_dma.h>
> > > > > > > > > > +#include <linux/platform_device.h>
> > > > > > > > > > +#include <linux/slab.h>
> > > > > > > > > > +
> > > > > > > > > > +#include "dmaengine.h"
> > > > > > > > > > +#include "virt-dma.h"
> > > > > > > > > > +
> > > > > > > > > > +/* Loongson-1 DMA Control Register */
> > > > > > > > > > +#define DMA_CTRL                       0x0
> > > > > > > > > > +
> > > > > > > > > > +/* DMA Control Register Bits */
> > > > > > > > > > +#define DMA_STOP                       BIT(4)
> > > > > > > > > > +#define DMA_START                      BIT(3)
> > > > > > > > > > +#define DMA_ASK_VALID                  BIT(2)
> > > > > > > > > > +
> > > > > > > > > > +#define DMA_ADDR_MASK                  GENMASK(31, 6)
> > > > > > > > > > +
> > > > > > > > > > +/* DMA Next Field Bits */
> > > > > > > > > > +#define DMA_NEXT_VALID                 BIT(0)
> > > > > > > > > > +
> > > > > > > > > > +/* DMA Command Field Bits */
> > > > > > > > > > +#define DMA_RAM2DEV                    BIT(12)
> > > > > > > > > > +#define DMA_INT                                BIT(1)
> > > > > > > > > > +#define DMA_INT_MASK                   BIT(0)
> > > > > > > > > > +
> > > > > > > > > > +#define LS1X_DMA_MAX_CHANNELS          3
> > > > > > > > > > +
> > > > > > > > > > +/* Size of allocations for hardware descriptors */
> > > > > > > > > > +#define LS1X_DMA_DESCS_SIZE            PAGE_SIZE
> > > > > > > > > > +#define LS1X_DMA_MAX_DESC              \
> > > > > > > > > > +       (LS1X_DMA_DESCS_SIZE / sizeof(struct ls1x_dma_hwdesc))
> > > > > > > > > > +
> > > > > > > > > > +struct ls1x_dma_hwdesc {
> > > > > > > > > > +       u32 next;               /* next descriptor address */
> > > > > > > > > > +       u32 saddr;              /* memory DMA address */
> > > > > > > > > > +       u32 daddr;              /* device DMA address */
> > > > > > > > > > +       u32 length;
> > > > > > > > > > +       u32 stride;
> > > > > > > > > > +       u32 cycles;
> > > > > > > > > > +       u32 cmd;
> > > > > > > > > > +       u32 stats;
> > > > > > > > > > +};
> > > > > > > > > > +
> > > > > > > > > > +struct ls1x_dma_desc {
> > > > > > > > > > +       struct virt_dma_desc vdesc;
> > > > > > > > > > +       enum dma_transfer_direction dir;
> > > > > > > > > > +       enum dma_transaction_type type;
> > > > > > > > > > +       unsigned int bus_width;
> > > > > > > > > > +
> > > > > > > > > > +       unsigned int nr_descs;  /* number of descriptors */
> > > > > > > > > > +
> > > > > > > > > > +       struct ls1x_dma_hwdesc *hwdesc;
> > > > > > > > > > +       dma_addr_t hwdesc_phys;
> > > > > > > > > > +};
> > > > > > > > > > +
> > > > > > > > > > +struct ls1x_dma_chan {
> > > > > > > > > > +       struct virt_dma_chan vchan;
> > > > > > > > > > +       struct dma_pool *desc_pool;
> > > > > > > > > > +       phys_addr_t src_addr;
> > > > > > > > > > +       phys_addr_t dst_addr;
> > > > > > > > > > +       enum dma_slave_buswidth src_addr_width;
> > > > > > > > > > +       enum dma_slave_buswidth dst_addr_width;
> > > > > > > > > > +
> > > > > > > > > > +       void __iomem *reg_base;
> > > > > > > > > > +       int irq;
> > > > > > > > > > +
> > > > > > > > > > +       struct ls1x_dma_desc *desc;
> > > > > > > > > > +
> > > > > > > > > > +       struct ls1x_dma_hwdesc *curr_hwdesc;
> > > > > > > > > > +       dma_addr_t curr_hwdesc_phys;
> > > > > > > > > > +};
> > > > > > > > > > +
> > > > > > > > > > +struct ls1x_dma {
> > > > > > > > > > +       struct dma_device ddev;
> > > > > > > > > > +       void __iomem *reg_base;
> > > > > > > > > > +
> > > > > > > > > > +       unsigned int nr_chans;
> > > > > > > > > > +       struct ls1x_dma_chan chan[];
> > > > > > > > > > +};
> > > > > > > > > > +
> > > > > > > > > > +#define to_ls1x_dma_chan(dchan)                \
> > > > > > > > > > +       container_of(dchan, struct ls1x_dma_chan, vchan.chan)
> > > > > > > > > > +
> > > > > > > > > > +#define to_ls1x_dma_desc(vd)           \
> > > > > > > > > > +       container_of(vd, struct ls1x_dma_desc, vdesc)
> > > > > > > > > > +
> > > > > > > > > > +/* macros for registers read/write */
> > > > > > > > > > +#define chan_readl(chan, off)          \
> > > > > > > > > > +       readl((chan)->reg_base + (off))
> > > > > > > > > > +
> > > > > > > > > > +#define chan_writel(chan, off, val)    \
> > > > > > > > > > +       writel((val), (chan)->reg_base + (off))
> > > > > > > > > > +
> > > > > > > > > > +static inline struct device *chan2dev(struct dma_chan *chan)
> > > > > > > > > > +{
> > > > > > > > > > +       return &chan->dev->device;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static inline int ls1x_dma_query(struct ls1x_dma_chan *chan,
> > > > > > > > > > +                                dma_addr_t *hwdesc_phys)
> > > > > > > > > > +{
> > > > > > > > > > +       struct dma_chan *dchan = &chan->vchan.chan;
> > > > > > > > > > +       int val, ret;
> > > > > > > > > > +
> > > > > > > > > > +       val = *hwdesc_phys & DMA_ADDR_MASK;
> > > > > > > > > > +       val |= DMA_ASK_VALID;
> > > > > > > > > > +       val |= dchan->chan_id;
> > > > > > > > > > +       chan_writel(chan, DMA_CTRL, val);
> > > > > > > > > > +       ret = readl_poll_timeout_atomic(chan->reg_base + DMA_CTRL, val,
> > > > > > > > > > +                                       !(val & DMA_ASK_VALID), 0, 3000);
> > > > > > > > > > +       if (ret)
> > > > > > > > > > +               dev_err(chan2dev(dchan), "failed to query DMA\n");
> > > > > > > > > > +
> > > > > > > > > > +       return ret;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static inline int ls1x_dma_start(struct ls1x_dma_chan *chan,
> > > > > > > > > > +                                dma_addr_t *hwdesc_phys)
> > > > > > > > > > +{
> > > > > > > > > > +       struct dma_chan *dchan = &chan->vchan.chan;
> > > > > > > > > > +       int val, ret;
> > > > > > > > > > +
> > > > > > > > > > +       dev_dbg(chan2dev(dchan), "cookie=%d, starting hwdesc=%x\n",
> > > > > > > > > > +               dchan->cookie, *hwdesc_phys);
> > > > > > > > > > +
> > > > > > > > > > +       val = *hwdesc_phys & DMA_ADDR_MASK;
> > > > > > > > > > +       val |= DMA_START;
> > > > > > > > > > +       val |= dchan->chan_id;
> > > > > > > > > > +       chan_writel(chan, DMA_CTRL, val);
> > > > > > > > > > +       ret = readl_poll_timeout(chan->reg_base + DMA_CTRL, val,
> > > > > > > > > > +                                !(val & DMA_START), 0, 3000);
> > > > > > > > > > +       if (ret)
> > > > > > > > > > +               dev_err(chan2dev(dchan), "failed to start DMA\n");
> > > > > > > > > > +
> > > > > > > > > > +       return ret;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static inline void ls1x_dma_stop(struct ls1x_dma_chan *chan)
> > > > > > > > > > +{
> > > > > > > > > > +       chan_writel(chan, DMA_CTRL, chan_readl(chan, DMA_CTRL) | DMA_STOP);
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static void ls1x_dma_free_chan_resources(struct dma_chan *dchan)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > > > +
> > > > > > > > > > +       dma_free_coherent(chan2dev(dchan), sizeof(struct ls1x_dma_hwdesc),
> > > > > > > > > > +                         chan->curr_hwdesc, chan->curr_hwdesc_phys);
> > > > > > > > > > +       vchan_free_chan_resources(&chan->vchan);
> > > > > > > > > > +       dma_pool_destroy(chan->desc_pool);
> > > > > > > > > > +       chan->desc_pool = NULL;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static int ls1x_dma_alloc_chan_resources(struct dma_chan *dchan)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > > > +
> > > > > > > > > > +       chan->desc_pool = dma_pool_create(dma_chan_name(dchan),
> > > > > > > > > > +                                         chan2dev(dchan),
> > > > > > > > > > +                                         sizeof(struct ls1x_dma_hwdesc),
> > > > > > > > > > +                                         __alignof__(struct ls1x_dma_hwdesc),
> > > > > > > > > > +                                         0);
> > > > > > > > > > +       if (!chan->desc_pool)
> > > > > > > > > > +               return -ENOMEM;
> > > > > > > > > > +
> > > > > > > > > > +       /* allocate memory for querying current HW descriptor */
> > > > > > > > > > +       dma_set_coherent_mask(chan2dev(dchan), DMA_BIT_MASK(32));
> > > > > > > > > > +       chan->curr_hwdesc = dma_alloc_coherent(chan2dev(dchan),
> > > > > > > > > > +                                              sizeof(struct ls1x_dma_hwdesc),
> > > > > > > > > > +                                              &chan->curr_hwdesc_phys,
> > > > > > > > > > +                                              GFP_KERNEL);
> > > > > > > > > > +       if (!chan->curr_hwdesc)
> > > > > > > > > > +               return -ENOMEM;
> > > > > > > > > > +
> > > > > > > > > > +       return 0;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static void ls1x_dma_free_desc(struct virt_dma_desc *vdesc)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_desc *desc = to_ls1x_dma_desc(vdesc);
> > > > > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(vdesc->tx.chan);
> > > > > > > > > > +
> > > > > > > > > > +       dma_pool_free(chan->desc_pool, desc->hwdesc, desc->hwdesc_phys);
> > > > > > > > > > +       chan->desc = NULL;
> > > > > > > > > > +       kfree(desc);
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static struct ls1x_dma_desc *
> > > > > > > > > > +ls1x_dma_alloc_desc(struct dma_chan *dchan, int sg_len,
> > > > > > > > > > +                   enum dma_transfer_direction direction,
> > > > > > > > > > +                   enum dma_transaction_type type)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > > > +       struct ls1x_dma_desc *desc;
> > > > > > > > > > +
> > > > > > > > > > +       if (sg_len > LS1X_DMA_MAX_DESC) {
> > > > > > > > > > +               dev_err(chan2dev(dchan), "sg_len %u exceeds limit %lu",
> > > > > > > > > > +                       sg_len, LS1X_DMA_MAX_DESC);
> > > > > > > > > > +               return NULL;
> > > > > > > > > > +       }
> > > > > > > > > > +
> > > > > > > > > > +       desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
> > > > > > > > > > +       if (!desc)
> > > > > > > > > > +               return NULL;
> > > > > > > > > > +
> > > > > > > > > > +       /* allocate HW descriptors */
> > > > > > > > > > +       desc->hwdesc = dma_pool_zalloc(chan->desc_pool, GFP_NOWAIT,
> > > > > > > > > > +                                      &desc->hwdesc_phys);
> > > > > > > > > > +       if (!desc->hwdesc) {
> > > > > > > > > > +               dev_err(chan2dev(dchan), "failed to alloc HW descriptors\n");
> > > > > > > > > > +               ls1x_dma_free_desc(&desc->vdesc);
> > > > > > > > > > +               return NULL;
> > > > > > > > > > +       }
> > > > > > > > > > +
> > > > > > > > > > +       desc->dir = direction;
> > > > > > > > > > +       desc->type = type;
> > > > > > > > > > +       desc->nr_descs = sg_len;
> > > > > > > > > > +
> > > > > > > > > > +       return desc;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static int ls1x_dma_setup_hwdescs(struct dma_chan *dchan,
> > > > > > > > > > +                                 struct ls1x_dma_desc *desc,
> > > > > > > > > > +                                 struct scatterlist *sgl, unsigned int sg_len)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > > > +       dma_addr_t next_hwdesc_phys = desc->hwdesc_phys;
> > > > > > > > > > +
> > > > > > > > > > +       struct scatterlist *sg;
> > > > > > > > > > +       unsigned int dev_addr, cmd, i;
> > > > > > > > > > +
> > > > > > > > > > +       switch (desc->dir) {
> > > > > > > > > > +       case DMA_MEM_TO_DEV:
> > > > > > > > > > +               dev_addr = chan->dst_addr;
> > > > > > > > > > +               desc->bus_width = chan->dst_addr_width;
> > > > > > > > > > +               cmd = DMA_RAM2DEV | DMA_INT;
> > > > > > > > > > +               break;
> > > > > > > > > > +       case DMA_DEV_TO_MEM:
> > > > > > > > > > +               dev_addr = chan->src_addr;
> > > > > > > > > > +               desc->bus_width = chan->src_addr_width;
> > > > > > > > > > +               cmd = DMA_INT;
> > > > > > > > > > +               break;
> > > > > > > > > > +       default:
> > > > > > > > > > +               dev_err(chan2dev(dchan), "unsupported DMA direction: %s\n",
> > > > > > > > > > +                       dmaengine_get_direction_text(desc->dir));
> > > > > > > > > > +               return -EINVAL;
> > > > > > > > > > +       }
> > > > > > > > > > +
> > > > > > > > > > +       /* setup HW descriptors */
> > > > > > > > > > +       for_each_sg(sgl, sg, sg_len, i) {
> > > > > > > > > > +               dma_addr_t buf_addr = sg_dma_address(sg);
> > > > > > > > > > +               size_t buf_len = sg_dma_len(sg);
> > > > > > > > > > +               struct ls1x_dma_hwdesc *hwdesc = &desc->hwdesc[i];
> > > > > > > > > > +
> > > > > > > > > > +               if (!is_dma_copy_aligned(dchan->device, buf_addr, 0, buf_len)) {
> > > > > > > > > > +                       dev_err(chan2dev(dchan), "buffer is not aligned!\n");
> > > > > > > > > > +                       return -EINVAL;
> > > > > > > > > > +               }
> > > > > > > > > > +
> > > > > > > > > > +               hwdesc->saddr = buf_addr;
> > > > > > > > > > +               hwdesc->daddr = dev_addr;
> > > > > > > > > > +               hwdesc->length = buf_len / desc->bus_width;
> > > > > > > > > > +               hwdesc->stride = 0;
> > > > > > > > > > +               hwdesc->cycles = 1;
> > > > > > > > > > +               hwdesc->cmd = cmd;
> > > > > > > > > > +
> > > > > > > > > > +               if (i) {
> > > > > > > > > > +                       next_hwdesc_phys += sizeof(*hwdesc);
> > > > > > > > > > +                       desc->hwdesc[i - 1].next = next_hwdesc_phys
> > > > > > > > > > +                           | DMA_NEXT_VALID;
> > > > > > > > > > +               }
> > > > > > > > > > +       }
> > > > > > > > > > +
> > > > > > > > > > +       if (desc->type == DMA_CYCLIC)
> > > > > > > > > > +               desc->hwdesc[i - 1].next = desc->hwdesc_phys | DMA_NEXT_VALID;
> > > > > > > > > > +
> > > > > > > > > > +       for_each_sg(sgl, sg, sg_len, i) {
> > > > > > > > > > +               struct ls1x_dma_hwdesc *hwdesc = &desc->hwdesc[i];
> > > > > > > > > > +
> > > > > > > > > > +               print_hex_dump_debug("HW DESC: ", DUMP_PREFIX_OFFSET, 16, 4,
> > > > > > > > > > +                                    hwdesc, sizeof(*hwdesc), false);
> > > > > > > > > > +       }
> > > > > > > > > > +
> > > > > > > > > > +       return 0;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static struct dma_async_tx_descriptor *
> > > > > > > > > > +ls1x_dma_prep_slave_sg(struct dma_chan *dchan,
> > > > > > > > > > +                      struct scatterlist *sgl, unsigned int sg_len,
> > > > > > > > > > +                      enum dma_transfer_direction direction,
> > > > > > > > > > +                      unsigned long flags, void *context)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > > > +       struct ls1x_dma_desc *desc;
> > > > > > > > > > +
> > > > > > > > > > +       dev_dbg(chan2dev(dchan), "sg_len=%u flags=0x%lx dir=%s\n",
> > > > > > > > > > +               sg_len, flags, dmaengine_get_direction_text(direction));
> > > > > > > > > > +
> > > > > > > > > > +       desc = ls1x_dma_alloc_desc(dchan, sg_len, direction, DMA_SLAVE);
> > > > > > > > > > +       if (!desc)
> > > > > > > > > > +               return NULL;
> > > > > > > > > > +
> > > > > > > > > > +       if (ls1x_dma_setup_hwdescs(dchan, desc, sgl, sg_len)) {
> > > > > > > > > > +               ls1x_dma_free_desc(&desc->vdesc);
> > > > > > > > > > +               return NULL;
> > > > > > > > > > +       }
> > > > > > > > > > +
> > > > > > > > > > +       return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags);
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static struct dma_async_tx_descriptor *
> > > > > > > > > > +ls1x_dma_prep_dma_cyclic(struct dma_chan *dchan,
> > > > > > > > > > +                        dma_addr_t buf_addr, size_t buf_len, size_t period_len,
> > > > > > > > > > +                        enum dma_transfer_direction direction,
> > > > > > > > > > +                        unsigned long flags)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > > > +       struct ls1x_dma_desc *desc;
> > > > > > > > > > +       struct scatterlist *sgl;
> > > > > > > > > > +       unsigned int sg_len;
> > > > > > > > > > +       unsigned int i;
> > > > > > > > > > +
> > > > > > > > > > +       dev_dbg(chan2dev(dchan),
> > > > > > > > > > +               "buf_len=%d period_len=%zu flags=0x%lx dir=%s\n", buf_len,
> > > > > > > > > > +               period_len, flags, dmaengine_get_direction_text(direction));
> > > > > > > > > > +
> > > > > > > > > > +       sg_len = buf_len / period_len;
> > > > > > > > > > +       desc = ls1x_dma_alloc_desc(dchan, sg_len, direction, DMA_CYCLIC);
> > > > > > > > > > +       if (!desc)
> > > > > > > > > > +               return NULL;
> > > > > > > > > > +
> > > > > > > > > > +       /* allocate the scatterlist */
> > > > > > > > > > +       sgl = kmalloc_array(sg_len, sizeof(*sgl), GFP_NOWAIT);
> > > > > > > > > > +       if (!sgl)
> > > > > > > > > > +               return NULL;
> > > > > > > > > > +
> > > > > > > > > > +       sg_init_table(sgl, sg_len);
> > > > > > > > > > +       for (i = 0; i < sg_len; ++i) {
> > > > > > > > > > +               sg_set_page(&sgl[i], pfn_to_page(PFN_DOWN(buf_addr)),
> > > > > > > > > > +                           period_len, offset_in_page(buf_addr));
> > > > > > > > > > +               sg_dma_address(&sgl[i]) = buf_addr;
> > > > > > > > > > +               sg_dma_len(&sgl[i]) = period_len;
> > > > > > > > > > +               buf_addr += period_len;
> > > > > > > > > > +       }
> > > > > > > > > > +
> > > > > > > > > > +       if (ls1x_dma_setup_hwdescs(dchan, desc, sgl, sg_len)) {
> > > > > > > > > > +               ls1x_dma_free_desc(&desc->vdesc);
> > > > > > > > > > +               return NULL;
> > > > > > > > > > +       }
> > > > > > > > > > +
> > > > > > > > > > +       kfree(sgl);
> > > > > > > > > > +
> > > > > > > > > > +       return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags);
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static int ls1x_dma_slave_config(struct dma_chan *dchan,
> > > > > > > > > > +                                struct dma_slave_config *config)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > > > +
> > > > > > > > > > +       chan->src_addr = config->src_addr;
> > > > > > > > > > +       chan->src_addr_width = config->src_addr_width;
> > > > > > > > > > +       chan->dst_addr = config->dst_addr;
> > > > > > > > > > +       chan->dst_addr_width = config->dst_addr_width;
> > > > > > > > > > +
> > > > > > > > > > +       return 0;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static int ls1x_dma_pause(struct dma_chan *dchan)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > > > +       unsigned long flags;
> > > > > > > > > > +       int ret;
> > > > > > > > > > +
> > > > > > > > > > +       spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > > > > +       ret = ls1x_dma_query(chan, &chan->curr_hwdesc_phys);
> > > > > > > > > > +       if (!ret)
> > > > > > > > > > +               ls1x_dma_stop(chan);
> > > > > > > > > > +       spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > > > > +
> > > > > > > > > > +       return ret;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static int ls1x_dma_resume(struct dma_chan *dchan)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > > > +       unsigned long flags;
> > > > > > > > > > +       int ret;
> > > > > > > > > > +
> > > > > > > > > > +       spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > > > > +       ret = ls1x_dma_start(chan, &chan->curr_hwdesc_phys);
> > > > > > > > > > +       spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > > > > +
> > > > > > > > > > +       return ret;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static int ls1x_dma_terminate_all(struct dma_chan *dchan)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > > > +       unsigned long flags;
> > > > > > > > > > +       LIST_HEAD(head);
> > > > > > > > > > +
> > > > > > > > > > +       spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > > > > +       ls1x_dma_stop(chan);
> > > > > > > > > > +       vchan_get_all_descriptors(&chan->vchan, &head);
> > > > > > > > > > +       spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > > > > +
> > > > > > > > > > +       vchan_dma_desc_free_list(&chan->vchan, &head);
> > > > > > > > > > +
> > > > > > > > > > +       return 0;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
> > > > > > > > > > +                                         dma_cookie_t cookie,
> > > > > > > > > > +                                         struct dma_tx_state *state)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > > > +       struct virt_dma_desc *vdesc;
> > > > > > > > > > +       enum dma_status status;
> > > > > > > > > > +       size_t bytes = 0;
> > > > > > > > > > +       unsigned long flags;
> > > > > > > > > > +
> > > > > > > > > > +       status = dma_cookie_status(dchan, cookie, state);
> > > > > > > > > > +       if (status == DMA_COMPLETE)
> > > > > > > > > > +               return status;
> > > > > > > > > > +
> > > > > > > > > > +       spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > > > > +       vdesc = vchan_find_desc(&chan->vchan, cookie);
> > > > > > > > > > +       if (chan->desc && cookie == chan->desc->vdesc.tx.cookie) {
> > > > > > > > > > +               struct ls1x_dma_desc *desc = chan->desc;
> > > > > > > > > > +               int i;
> > > > > > > > > > +
> > > > > > > > > > +               if (ls1x_dma_query(chan, &chan->curr_hwdesc_phys))
> > > > > > > > > > +                       return status;
> > > > > > > > > > +
> > > > > > > > > > +               /* locate the current HW descriptor */
> > > > > > > > > > +               for (i = 0; i < desc->nr_descs; i++)
> > > > > > > > > > +                       if (desc->hwdesc[i].next == chan->curr_hwdesc->next)
> > > > > > > > > > +                               break;
> > > > > > > > > > +
> > > > > > > > > > +               /* count the residues */
> > > > > > > > > > +               for (; i < desc->nr_descs; i++)
> > > > > > > > > > +                       bytes += desc->hwdesc[i].length * desc->bus_width;
> > > > > > > > > > +
> > > > > > > > > > +               dma_set_residue(state, bytes);
> > > > > > > > > > +       }
> > > > > > > > > > +       spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > > > > +
> > > > > > > > > > +       return status;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static void ls1x_dma_issue_pending(struct dma_chan *dchan)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > > > +       struct virt_dma_desc *vdesc;
> > > > > > > > > > +       unsigned long flags;
> > > > > > > > > > +
> > > > > > > > > > +       spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > > > > +       if (vchan_issue_pending(&chan->vchan) && !chan->desc) {
> > > > > > > > > > +               vdesc = vchan_next_desc(&chan->vchan);
> > > > > > > > > > +               if (!vdesc) {
> > > > > > > > > > +                       chan->desc = NULL;
> > > > > > > > > > +                       return;
> > > > > > > > > > +               }
> > > > > > > > > > +               chan->desc = to_ls1x_dma_desc(vdesc);
> > > > > > > > > > +               ls1x_dma_start(chan, &chan->desc->hwdesc_phys);
> > > > > > > > > > +       }
> > > > > > > > > > +       spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static irqreturn_t ls1x_dma_irq_handler(int irq, void *data)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma_chan *chan = data;
> > > > > > > > > > +       struct ls1x_dma_desc *desc = chan->desc;
> > > > > > > > > > +       struct dma_chan *dchan = &chan->vchan.chan;
> > > > > > > > > > +
> > > > > > > > > > +       if (!desc) {
> > > > > > > > > > +               dev_warn(chan2dev(dchan),
> > > > > > > > > > +                        "IRQ %d with no active descriptor on channel %d\n",
> > > > > > > > > > +                        irq, dchan->chan_id);
> > > > > > > > > > +               return IRQ_NONE;
> > > > > > > > > > +       }
> > > > > > > > > > +
> > > > > > > > > > +       dev_dbg(chan2dev(dchan), "DMA IRQ %d on channel %d\n", irq,
> > > > > > > > > > +               dchan->chan_id);
> > > > > > > > > > +
> > > > > > > > > > +       spin_lock(&chan->vchan.lock);
> > > > > > > > > > +
> > > > > > > > > > +       if (desc->type == DMA_CYCLIC) {
> > > > > > > > > > +               vchan_cyclic_callback(&desc->vdesc);
> > > > > > > > > > +       } else {
> > > > > > > > > > +               list_del(&desc->vdesc.node);
> > > > > > > > > > +               vchan_cookie_complete(&desc->vdesc);
> > > > > > > > > > +               chan->desc = NULL;
> > > > > > > > > > +       }
> > > > > > > > > > +
> > > > > > > > > > +       spin_unlock(&chan->vchan.lock);
> > > > > > > > > > +       return IRQ_HANDLED;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static int ls1x_dma_chan_probe(struct platform_device *pdev,
> > > > > > > > > > +                              struct ls1x_dma *dma, int chan_id)
> > > > > > > > > > +{
> > > > > > > > > > +       struct device *dev = &pdev->dev;
> > > > > > > > > > +       struct ls1x_dma_chan *chan = &dma->chan[chan_id];
> > > > > > > > > > +       char pdev_irqname[4];
> > > > > > > > > > +       char *irqname;
> > > > > > > > > > +       int ret;
> > > > > > > > > > +
> > > > > > > > > > +       sprintf(pdev_irqname, "ch%u", chan_id);
> > > > > > > > > > +       chan->irq = platform_get_irq_byname(pdev, pdev_irqname);
> > > > > > > > > > +       if (chan->irq < 0)
> > > > > > > > > > +               return -ENODEV;
> > > > > > > > > > +
> > > > > > > > > > +       irqname = devm_kasprintf(dev, GFP_KERNEL, "%s:%s",
> > > > > > > > > > +                                dev_name(dev), pdev_irqname);
> > > > > > > > > > +       if (!irqname)
> > > > > > > > > > +               return -ENOMEM;
> > > > > > > > > > +
> > > > > > > > > > +       ret = devm_request_irq(dev, chan->irq, ls1x_dma_irq_handler,
> > > > > > > > > > +                              IRQF_SHARED, irqname, chan);
> > > > > > > > > > +       if (ret)
> > > > > > > > > > +               return dev_err_probe(dev, ret,
> > > > > > > > > > +                                    "failed to request IRQ %u!\n", chan->irq);
> > > > > > > > > > +
> > > > > > > > > > +       chan->reg_base = dma->reg_base;
> > > > > > > > > > +       chan->vchan.desc_free = ls1x_dma_free_desc;
> > > > > > > > > > +       vchan_init(&chan->vchan, &dma->ddev);
> > > > > > > > > > +       dev_info(dev, "%s (irq %d) initialized\n", pdev_irqname, chan->irq);
> > > > > > > > > > +
> > > > > > > > > > +       return 0;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static void ls1x_dma_chan_remove(struct ls1x_dma *dma, int chan_id)
> > > > > > > > > > +{
> > > > > > > > > > +       struct device *dev = dma->ddev.dev;
> > > > > > > > > > +       struct ls1x_dma_chan *chan = &dma->chan[chan_id];
> > > > > > > > > > +
> > > > > > > > > > +       devm_free_irq(dev, chan->irq, chan);
> > > > > > > > > > +       list_del(&chan->vchan.chan.device_node);
> > > > > > > > > > +       tasklet_kill(&chan->vchan.task);
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static int ls1x_dma_probe(struct platform_device *pdev)
> > > > > > > > > > +{
> > > > > > > > > > +       struct device *dev = &pdev->dev;
> > > > > > > > > > +       struct dma_device *ddev;
> > > > > > > > > > +       struct ls1x_dma *dma;
> > > > > > > > > > +       int nr_chans, ret, i;
> > > > > > > > > > +
> > > > > > > > > > +       nr_chans = platform_irq_count(pdev);
> > > > > > > > > > +       if (nr_chans <= 0)
> > > > > > > > > > +               return nr_chans;
> > > > > > > > > > +       if (nr_chans > LS1X_DMA_MAX_CHANNELS)
> > > > > > > > > > +               return dev_err_probe(dev, -EINVAL,
> > > > > > > > > > +                                    "nr_chans=%d exceeds the maximum\n",
> > > > > > > > > > +                                    nr_chans);
> > > > > > > > > > +
> > > > > > > > > > +       dma = devm_kzalloc(dev, struct_size(dma, chan, nr_chans), GFP_KERNEL);
> > > > > > > > > > +       if (!dma)
> > > > > > > > > > +               return -ENOMEM;
> > > > > > > > > > +
> > > > > > > > > > +       /* initialize DMA device */
> > > > > > > > > > +       dma->reg_base = devm_platform_ioremap_resource(pdev, 0);
> > > > > > > > > > +       if (IS_ERR(dma->reg_base))
> > > > > > > > > > +               return PTR_ERR(dma->reg_base);
> > > > > > > > > > +
> > > > > > > > > > +       ddev = &dma->ddev;
> > > > > > > > > > +       ddev->dev = dev;
> > > > > > > > > > +       ddev->copy_align = DMAENGINE_ALIGN_4_BYTES;
> > > > > > > > > > +       ddev->src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> > > > > > > > > > +           BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> > > > > > > > > > +       ddev->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> > > > > > > > > > +           BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> > > > > > > > > > +       ddev->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> > > > > > > > > > +       ddev->max_sg_burst = LS1X_DMA_MAX_DESC;
> > > > > > > > > > +       ddev->residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
> > > > > > > > > > +       ddev->device_alloc_chan_resources = ls1x_dma_alloc_chan_resources;
> > > > > > > > > > +       ddev->device_free_chan_resources = ls1x_dma_free_chan_resources;
> > > > > > > > > > +       ddev->device_prep_slave_sg = ls1x_dma_prep_slave_sg;
> > > > > > > > > > +       ddev->device_prep_dma_cyclic = ls1x_dma_prep_dma_cyclic;
> > > > > > > > > > +       ddev->device_config = ls1x_dma_slave_config;
> > > > > > > > > > +       ddev->device_pause = ls1x_dma_pause;
> > > > > > > > > > +       ddev->device_resume = ls1x_dma_resume;
> > > > > > > > > > +       ddev->device_terminate_all = ls1x_dma_terminate_all;
> > > > > > > > > > +       ddev->device_tx_status = ls1x_dma_tx_status;
> > > > > > > > > > +       ddev->device_issue_pending = ls1x_dma_issue_pending;
> > > > > > > > > > +
> > > > > > > > > > +       dma_cap_set(DMA_SLAVE, ddev->cap_mask);
> > > > > > > > > > +       INIT_LIST_HEAD(&ddev->channels);
> > > > > > > > > > +
> > > > > > > > > > +       /* initialize DMA channels */
> > > > > > > > > > +       for (i = 0; i < nr_chans; i++) {
> > > > > > > > > > +               ret = ls1x_dma_chan_probe(pdev, dma, i);
> > > > > > > > > > +               if (ret)
> > > > > > > > > > +                       return ret;
> > > > > > > > > > +       }
> > > > > > > > > > +       dma->nr_chans = nr_chans;
> > > > > > > > > > +
> > > > > > > > > > +       ret = dmaenginem_async_device_register(ddev);
> > > > > > > > > > +       if (ret) {
> > > > > > > > > > +               dev_err(dev, "failed to register DMA device! %d\n", ret);
> > > > > > > > > > +               return ret;
> > > > > > > > > > +       }
> > > > > > > > > > +
> > > > > > > > > > +       ret =
> > > > > > > > > > +           of_dma_controller_register(dev->of_node, of_dma_xlate_by_chan_id,
> > > > > > > > > > +                                      ddev);
> > > > > > > > > > +       if (ret) {
> > > > > > > > > > +               dev_err(dev, "failed to register DMA controller! %d\n", ret);
> > > > > > > > > > +               return ret;
> > > > > > > > > > +       }
> > > > > > > > > > +
> > > > > > > > > > +       platform_set_drvdata(pdev, dma);
> > > > > > > > > > +       dev_info(dev, "Loongson1 DMA driver registered\n");
> > > > > > > > > > +
> > > > > > > > > > +       return 0;
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static void ls1x_dma_remove(struct platform_device *pdev)
> > > > > > > > > > +{
> > > > > > > > > > +       struct ls1x_dma *dma = platform_get_drvdata(pdev);
> > > > > > > > > > +       int i;
> > > > > > > > > > +
> > > > > > > > > > +       of_dma_controller_free(pdev->dev.of_node);
> > > > > > > > > > +
> > > > > > > > > > +       for (i = 0; i < dma->nr_chans; i++)
> > > > > > > > > > +               ls1x_dma_chan_remove(dma, i);
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static const struct of_device_id ls1x_dma_match[] = {
> > > > > > > > > > +       { .compatible = "loongson,ls1b-apbdma" },
> > > > > > > > > > +       { .compatible = "loongson,ls1c-apbdma" },
> > > > > > > > > > +       { /* sentinel */ }
> > > > > > > > > > +};
> > > > > > > > > > +MODULE_DEVICE_TABLE(of, ls1x_dma_match);
> > > > > > > > > > +
> > > > > > > > > > +static struct platform_driver ls1x_dma_driver = {
> > > > > > > > > > +       .probe = ls1x_dma_probe,
> > > > > > > > > > +       .remove_new = ls1x_dma_remove,
> > > > > > > > > > +       .driver = {
> > > > > > > > > > +               .name = KBUILD_MODNAME,
> > > > > > > > > > +               .of_match_table = ls1x_dma_match,
> > > > > > > > > > +       },
> > > > > > > > > > +};
> > > > > > > > > > +
> > > > > > > > > > +module_platform_driver(ls1x_dma_driver);
> > > > > > > > > > +
> > > > > > > > > > +MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");
> > > > > > > > > > +MODULE_DESCRIPTION("Loongson-1 APB DMA Controller driver");
> > > > > > > > > > +MODULE_LICENSE("GPL");
> > > > > > > > > >
> > > > > > > > > > --
> > > > > > > > > > 2.40.1
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Best regards,
> > > > > > > >
> > > > > > > > Keguang Zhang
> > > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Best regards,
> > > > > >
> > > > > > Keguang Zhang
> > > >
> > > >
> > > >
> > > > --
> > > > Best regards,
> > > >
> > > > Keguang Zhang
> >
> >
> >
> > --
> > Best regards,
> >
> > Keguang Zhang



-- 
Best regards,

Keguang Zhang

^ permalink raw reply

* [PATCH v2] dt-bindings: watchdog: aspeed,ast2400-wdt: Convert to DT schema
From: Andrew Jeffery @ 2024-04-03  2:04 UTC (permalink / raw)
  To: wim, linux
  Cc: Andrew Jeffery, robh, krzysztof.kozlowski+dt, conor+dt, joel, zev,
	linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel

Squash warnings such as:

```
arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-galaxy100.dtb: /ahb/apb@1e600000/watchdog@1e785000: failed to match any schema with compatible: ['aspeed,ast2400-wdt']
```

The schema binding additionally defines the clocks property over the
prose binding to align with use of the node in the DTS files.

Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
---
v2: Address feedback from Rob and Zev

    - Rob: https://lore.kernel.org/linux-watchdog/20240402180718.GA358505-robh@kernel.org/
    - Zev: https://lore.kernel.org/linux-watchdog/65722a59-2e94-4616-81e1-835615b0e600@hatter.bewilderbeest.net/

v1: https://lore.kernel.org/linux-watchdog/20240402120118.282035-1-andrew@codeconstruct.com.au/

 .../bindings/watchdog/aspeed,ast2400-wdt.yaml | 142 ++++++++++++++++++
 .../bindings/watchdog/aspeed-wdt.txt          |  73 ---------
 2 files changed, 142 insertions(+), 73 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml
 delete mode 100644 Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml
new file mode 100644
index 000000000000..be78a9865584
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml
@@ -0,0 +1,142 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/aspeed,ast2400-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Aspeed watchdog timer controllers
+
+maintainers:
+  - Andrew Jeffery <andrew@codeconstruct.com.au>
+
+properties:
+  compatible:
+    enum:
+      - aspeed,ast2400-wdt
+      - aspeed,ast2500-wdt
+      - aspeed,ast2600-wdt
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description: >
+      The clock used to drive the watchdog counter. From the AST2500 no source
+      other than the 1MHz clock can be selected, so the clocks property is
+      optional.
+
+  aspeed,reset-type:
+    $ref: /schemas/types.yaml#/definitions/string
+    enum:
+      - cpu
+      - soc
+      - system
+      - none
+    default: system
+    description: >
+      The watchdog can be programmed to generate one of three different types of
+      reset when a timeout occcurs.
+
+      Specifying 'cpu' will only reset the processor on a timeout event.
+
+      Specifying 'soc' will reset a configurable subset of the SoC's controllers
+      on a timeout event. Controllers critical to the SoC's operation may remain
+      untouched. The set of SoC controllers to reset may be specified via the
+      aspeed,reset-mask property if the node has the aspeed,ast2500-wdt or
+      aspeed,ast2600-wdt compatible.
+
+      Specifying 'system' will reset all controllers on a timeout event, as if
+      EXTRST had been asserted.
+
+      Specifying 'none' will cause the timeout event to have no reset effect.
+      Another watchdog engine on the chip must be used for chip reset operations.
+
+  aspeed,alt-boot:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: >
+      Direct the watchdog to configure the SoC to boot from the alternative boot
+      region if a timeout occurs.
+
+  aspeed,external-signal:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: >
+      Assert the timeout event on an external signal pin associated with the
+      watchdog controller instance. The pin must be muxed appropriately.
+
+  aspeed,ext-pulse-duration:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: >
+      The duration, in microseconds, of the pulse emitted on the external signal
+      pin.
+
+  aspeed,ext-push-pull:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: >
+      If aspeed,external-signal is specified in the node, set the external
+      signal pin's drive type to push-pull. If aspeed,ext-push-pull is not
+      specified then the pin is configured as open-drain.
+
+  aspeed,ext-active-high:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: >
+      If both aspeed,external-signal and aspeed,ext-push-pull are specified in
+      the node, set the pulse polarity to active-high. If aspeed,ext-active-high
+      is not specified then the pin is configured as active-low.
+
+  aspeed,reset-mask:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 2
+    description: >
+      A bitmask indicating which peripherals will be reset if the watchdog
+      timer expires. On AST2500 SoCs this should be a single word defined using
+      the AST2500_WDT_RESET_* macros; on AST2600 SoCs this should be a two-word
+      array with the first word defined using the AST2600_WDT_RESET1_* macros,
+      and the second word defined using the AST2600_WDT_RESET2_* macros.
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - if:
+      anyOf:
+        - required:
+            - aspeed,ext-push-pull
+        - required:
+            - aspeed,ext-active-high
+        - required:
+            - aspeed,reset-mask
+    then:
+      properties:
+        compatible:
+          enum:
+            - aspeed,ast2500-wdt
+            - aspeed,ast2600-wdt
+  - if:
+      required:
+        - aspeed,ext-active-high
+    then:
+      required:
+        - aspeed,ext-push-pull
+
+additionalProperties: false
+
+examples:
+  - |
+    watchdog@1e785000 {
+        compatible = "aspeed,ast2400-wdt";
+        reg = <0x1e785000 0x1c>;
+        aspeed,reset-type = "system";
+        aspeed,external-signal;
+    };
+  - |
+    #include <dt-bindings/watchdog/aspeed-wdt.h>
+    watchdog@1e785040 {
+        compatible = "aspeed,ast2600-wdt";
+        reg = <0x1e785040 0x40>;
+        aspeed,reset-type = "soc";
+        aspeed,reset-mask = <AST2600_WDT_RESET1_DEFAULT
+                            (AST2600_WDT_RESET2_DEFAULT & ~AST2600_WDT_RESET2_LPC)>;
+    };
diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
deleted file mode 100644
index 3208adb3e52e..000000000000
--- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
+++ /dev/null
@@ -1,73 +0,0 @@
-Aspeed Watchdog Timer
-
-Required properties:
- - compatible: must be one of:
-	- "aspeed,ast2400-wdt"
-	- "aspeed,ast2500-wdt"
-	- "aspeed,ast2600-wdt"
-
- - reg: physical base address of the controller and length of memory mapped
-   region
-
-Optional properties:
-
- - aspeed,reset-type = "cpu|soc|system|none"
-
-   Reset behavior - Whenever a timeout occurs the watchdog can be programmed
-   to generate one of three different, mutually exclusive, types of resets.
-
-   Type "none" can be specified to indicate that no resets are to be done.
-   This is useful in situations where another watchdog engine on chip is
-   to perform the reset.
-
-   If 'aspeed,reset-type=' is not specified the default is to enable system
-   reset.
-
-   Reset types:
-
-        - cpu: Reset CPU on watchdog timeout
-
-        - soc: Reset 'System on Chip' on watchdog timeout
-
-        - system: Reset system on watchdog timeout
-
-        - none: No reset is performed on timeout. Assumes another watchdog
-                engine is responsible for this.
-
- - aspeed,alt-boot:    If property is present then boot from alternate block.
- - aspeed,external-signal: If property is present then signal is sent to
-			external reset counter (only WDT1 and WDT2). If not
-			specified no external signal is sent.
- - aspeed,ext-pulse-duration: External signal pulse duration in microseconds
-
-Optional properties for AST2500-compatible watchdogs:
- - aspeed,ext-push-pull: If aspeed,external-signal is present, set the pin's
-			 drive type to push-pull. The default is open-drain.
- - aspeed,ext-active-high: If aspeed,external-signal is present and and the pin
-			   is configured as push-pull, then set the pulse
-			   polarity to active-high. The default is active-low.
-
-Optional properties for AST2500- and AST2600-compatible watchdogs:
- - aspeed,reset-mask: A bitmask indicating which peripherals will be reset if
-		      the watchdog timer expires.  On AST2500 this should be a
-		      single word defined using the AST2500_WDT_RESET_* macros;
-		      on AST2600 this should be a two-word array with the first
-		      word defined using the AST2600_WDT_RESET1_* macros and the
-		      second word defined using the AST2600_WDT_RESET2_* macros.
-
-Examples:
-
-	wdt1: watchdog@1e785000 {
-		compatible = "aspeed,ast2400-wdt";
-		reg = <0x1e785000 0x1c>;
-		aspeed,reset-type = "system";
-		aspeed,external-signal;
-	};
-
-	#include <dt-bindings/watchdog/aspeed-wdt.h>
-	wdt2: watchdog@1e785040 {
-		compatible = "aspeed,ast2600-wdt";
-		reg = <0x1e785040 0x40>;
-		aspeed,reset-mask = <AST2600_WDT_RESET1_DEFAULT
-				     (AST2600_WDT_RESET2_DEFAULT & ~AST2600_WDT_RESET2_LPC)>;
-	};
-- 
2.39.2


^ permalink raw reply related

* Re: [PATCH] arch: arm: mxc: phyCARD-i.MX27: Add USB support
From: Shawn Guo @ 2024-04-03  1:50 UTC (permalink / raw)
  To: Michael Grzeschik
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, devicetree,
	imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240328-pca100-v1-1-58df67c2c950@pengutronix.de>

On Tue, Apr 02, 2024 at 09:35:33AM +0200, Michael Grzeschik wrote:
> This patch adds the pinmux and nodes for usbotg and usbh2.
> 
> In v6 revision of the pca100 the usb phys were changed to usb3320 which
> are connected by their reset pins. We add the phy configuration to the
> description.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

"ARM: dts: ..." for subject prefix.

Shawn


^ permalink raw reply

* Re: [PATCH v5 0/4] arm64: dts: imx8: add cm40 and cm40_uart
From: Shawn Guo @ 2024-04-03  1:45 UTC (permalink / raw)
  To: Frank Li
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, devicetree,
	imx, linux-arm-kernel, linux-kernel, Dong Aisheng, Peng Fan,
	Alexander Stein, Alice Guo
In-Reply-To: <20240402-m4_lpuart-v5-0-3292629ba808@nxp.com>

On Tue, Apr 02, 2024 at 10:41:27AM -0400, Frank Li wrote:
> Alice Guo (1):
>       arm64: dts: imx8dxl: add lpuart device in cm40 subsystem
> 
> Dong Aisheng (1):
>       arm64: dts: imx8: add cm40 subsystem dtsi
> 
> Frank Li (2):
>       arm64: dts: imx8dxl: update cm40 irq number information
>       dts: arm64: imx8dxl-evk: add lpuart1 and cm40 uart

arm64: dts: imx8dxl-evk: ...

Fixed it up and applied the series, thanks!

Shawn


^ permalink raw reply

* Re: [PATCH 1/7] arm64: dts: imx8-ss-lsio: fix pwm lpcg indices
From: Shawn Guo @ 2024-04-03  1:41 UTC (permalink / raw)
  To: Frank Li
  Cc: Fabio Estevam, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	Marcel Ziswiler, Philippe Schenker, Max Krummenacher,
	Alexander Stein, Joakim Zhang, devicetree, linux-arm-kernel,
	linux-kernel, stable
In-Reply-To: <ZgwfnZJDRYmYy7Qt@lizhi-Precision-Tower-5810>

On Tue, Apr 02, 2024 at 11:09:17AM -0400, Frank Li wrote:
> On Mon, Apr 01, 2024 at 08:04:56PM -0300, Fabio Estevam wrote:
> > On Mon, Apr 1, 2024 at 7:25 PM Frank Li <Frank.Li@nxp.com> wrote:
> > >
> > > lpcg's arg0 should use clock indices instead of index.
> > >
> > > pwm0_lpcg: clock-controller@5d400000 {
> > >         ...                                                // Col1  Col2
> > >         clocks = <&clk IMX_SC_R_PWM_0 IMX_SC_PM_CLK_PER>,  // 0     0
> > >                  <&clk IMX_SC_R_PWM_0 IMX_SC_PM_CLK_PER>,  // 1     1
> > >                  <&clk IMX_SC_R_PWM_0 IMX_SC_PM_CLK_PER>,  // 2     4
> > >                  <&lsio_bus_clk>,                          // 3     5
> > >                  <&clk IMX_SC_R_PWM_0 IMX_SC_PM_CLK_PER>;  // 4     6
> > >         clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_1>,
> > >                         <IMX_LPCG_CLK_4>, <IMX_LPCG_CLK_5>,
> > >                         <IMX_LPCG_CLK_6>;
> > > };
> > >
> > > Col1: index, which exited dts try to get.
> > 
> > I cannot understand this sentence, sorry.
> 
> This base on downstream dts code.  Downstream code use index in 'Col1' to
> get clock.

So s/exited/existing you meant?

Shawn


^ permalink raw reply

* Re: [net-next,v2] dt-bindings: net: renesas,ethertsn: Create child-node for MDIO bus
From: patchwork-bot+netdevbpf @ 2024-04-03  1:40 UTC (permalink / raw)
  To: =?utf-8?q?Niklas_S=C3=B6derlund_=3Cniklas=2Esoderlund+renesas=40ragnatech=2E?=,
	=?utf-8?q?se=3E?=
  Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, geert+renesas, netdev,
	devicetree, linux-renesas-soc, robh
In-Reply-To: <20240330131228.1541227-1-niklas.soderlund+renesas@ragnatech.se>

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sat, 30 Mar 2024 14:12:28 +0100 you wrote:
> The bindings for Renesas Ethernet TSN was just merged in v6.9 and the
> design for the bindings followed that of other Renesas Ethernet drivers
> and thus did not force a child-node for the MDIO bus. As there
> are no upstream drivers or users of this binding yet take the
> opportunity to correct this and force the usage of a child-node for the
> MDIO bus.
> 
> [...]

Here is the summary with links:
  - [net-next,v2] dt-bindings: net: renesas,ethertsn: Create child-node for MDIO bus
    https://git.kernel.org/netdev/net-next/c/8da891720cd4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply

* Re: [PATCH] arm64: dts: imx8m*-venice-gw7: Fix TPM schema violations
From: Shawn Guo @ 2024-04-03  1:20 UTC (permalink / raw)
  To: Tim Harvey
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, devicetree,
	imx, linux-arm-kernel, linux-kernel, Lukas Wunner
In-Reply-To: <20240402193355.2333597-1-tharvey@gateworks.com>

On Tue, Apr 02, 2024 at 12:33:55PM -0700, Tim Harvey wrote:
> Since commit 26c9d152ebf3 ("dt-bindings: tpm: Consolidate TCG TIS
> bindings"), several issues are reported by "make dtbs_check" for arm64
> devicetrees:
> 
> The compatible property needs to contain the chip's name in addition to
> the generic "tcg,tpm_tis-spi".
> 
> tpm@1: compatible: ['tcg,tpm_tis-spi'] is too short
> 	from schema $id:
> http://devicetree.org/schemas/tpm/tcg,tpm_tis-spi.yaml#
> 
> Fix these schema violations.
> 
> Gateworks Venice uses an Atmel ATTPM20P:
> https://trac.gateworks.com/wiki/tpm
> 
> Cc: Lukas Wunner <lukas@wunner.de>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>

Applied, thanks!


^ permalink raw reply

* Re: [PATCH v2 1/2] ARM: dts: imx6sx-nitrogen6sx: drop incorrect cpu-dai property
From: Shawn Guo @ 2024-04-03  1:18 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	shengjiu.wang, linux-sound, devicetree, linux-kernel, shawnguo,
	s.hauer, kernel, festevam, imx, linux-arm-kernel
In-Reply-To: <1711976056-19884-2-git-send-email-shengjiu.wang@nxp.com>

On Mon, Apr 01, 2024 at 08:54:15PM +0800, Shengjiu Wang wrote:
> drop incorrect cpu-dai property, change it to ssi-controller
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>

Applied, thanks!


^ permalink raw reply

* Re: [PATCH v2 0/3] Add support for Emcraft Systems NavQ+ kit
From: Shawn Guo @ 2024-04-03  1:15 UTC (permalink / raw)
  To: Gilles Talis
  Cc: devicetree, imx, linux-arm-kernel, conor+dt,
	krzysztof.kozlowski+dt, robh, shawnguo, festevam, alex, andrew
In-Reply-To: <20240330133410.41408-1-gilles.talis@gmail.com>

On Sat, Mar 30, 2024 at 09:34:07AM -0400, Gilles Talis wrote:
> Hello
> 
> This series adds a device tree file for the Emcraft Systems NavQ+ kit [1]
> 
> The first patch adds a new vendor prefix for Emcraft Systems
> The second one adds the board to the arm/fsl.yaml DT bindings.
> Last patch adds device tree file for the kit.
> 
> [1] https://www.emcraft.com/products/1222
> 
> Changes in v2:
> - Add Acked-by review tags
> - Fixed device tree warnings reported by dtbs_check
> - Reworked leds node
> - Remove unused i2c6 pinctrl entry
> - Removed unused regulator node in Ethernet entry
> - Link to v1: https://lore.kernel.org/imx/20240328202320.187596-1-gilles.talis@gmail.com/
> ---
> Gilles Talis (3):
>   dt-bindings: vendor-prefixes: Add Emcraft Systems
>   dt-bindings: arm: Add Emcraft Systems i.MX8M Plus NavQ+ Kit
>   arm64: dts: freescale: Add device tree for Emcraft Systems NavQ+ Kit

Applied all, thanks! 


^ permalink raw reply

* Re: [PATCH] arm64: dts: imx8m*-venice-gw7: Fix TPM schema violations
From: Fabio Estevam @ 2024-04-03  1:11 UTC (permalink / raw)
  To: Tim Harvey
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, devicetree, imx,
	linux-arm-kernel, linux-kernel, Lukas Wunner
In-Reply-To: <20240402193355.2333597-1-tharvey@gateworks.com>

Hi Tim,

On Tue, Apr 2, 2024 at 4:34 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> Since commit 26c9d152ebf3 ("dt-bindings: tpm: Consolidate TCG TIS
> bindings"), several issues are reported by "make dtbs_check" for arm64
> devicetrees:
>
> The compatible property needs to contain the chip's name in addition to
> the generic "tcg,tpm_tis-spi".
>
> tpm@1: compatible: ['tcg,tpm_tis-spi'] is too short
>         from schema $id:
> http://devicetree.org/schemas/tpm/tcg,tpm_tis-spi.yaml#
>
> Fix these schema violations.
>
> Gateworks Venice uses an Atmel ATTPM20P:
> https://trac.gateworks.com/wiki/tpm

Thanks for the fix.

Reviewed-by: Fabio Estevam <festevam@gmail.com>

^ permalink raw reply

* Re: Fixing the devicetree of Rock 5 Model B (and possibly others)
From: Pratham Patel @ 2024-04-03  1:03 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Dragan Simic, sebastian.reichel, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel, regressions, stable
In-Reply-To: <CAGETcx_ToHsp_c+Yt0qqST4Zd-GC7dPn_j=PpB1n1xpZtOnMfg@mail.gmail.com>

On Wed Apr 3, 2024 at 6:16 AM IST, Saravana Kannan wrote:
> On Tue, Apr 2, 2024 at 4:32 PM Pratham Patel
> <prathampatel@thefossguy.com> wrote:
> >
> > On Tue Apr 2, 2024 at 4:54 AM IST, Saravana Kannan wrote:
> > > On Sat, Mar 23, 2024 at 10:10 AM Dragan Simic <dsimic@manjaro.org> wrote:
> > > >
> > > > Hello Pratham,
> > > >
> > > > On 2024-03-23 18:02, Pratham Patel wrote:
> > > > > I looked at the patch and tried several things, neither resulted in
> > > > > anything that would point me to the core issue. Then I tried this:
> > > >
> > > > Could you, please, clarify a bit what's the actual issue you're
> > > > experiencing on your Rock 5B?
> > >
> > > Pratham, can you reply to this please? I don't really understand what
> > > your issue is for me to be able to help.
> >
> > Hi,
> >
> > I apologize for not replying. Somehow, I did not notice the reply from
> > Dragan. :(
> >
> > Since this patch was applied, an issue in the Rock 5B's DT has been
> > unearthed which now results in the kernel being unable to boot properly.
> >
> > Following is the relevant call trace from the UART capture:
> >
> > [   21.595068] Call trace:
> > [   21.595288]  smp_call_function_many_cond+0x174/0x5f8
> > [   21.595728]  on_each_cpu_cond_mask+0x2c/0x40
> > [   21.596109]  cpuidle_register_driver+0x294/0x318
> > [   21.596524]  cpuidle_register+0x24/0x100
> > [   21.596875]  psci_cpuidle_probe+0x2e4/0x490
> > [   21.597247]  platform_probe+0x70/0xd0
> > [   21.597575]  really_probe+0x18c/0x3d8
> > [   21.597905]  __driver_probe_device+0x84/0x180
> > [   21.598294]  driver_probe_device+0x44/0x120
> > [   21.598669]  __device_attach_driver+0xc4/0x168
> > [   21.599063]  bus_for_each_drv+0x8c/0xf0
> > [   21.599408]  __device_attach+0xa4/0x1c0
> > [   21.599748]  device_initial_probe+0x1c/0x30
> > [   21.600118]  bus_probe_device+0xb4/0xc0
> > [   21.600462]  device_add+0x68c/0x888
> > [   21.600775]  platform_device_add+0x19c/0x270
> > [   21.601154]  platform_device_register_full+0xdc/0x178
> > [   21.601602]  psci_idle_init+0xa0/0xc8
> > [   21.601934]  do_one_initcall+0x60/0x290
> > [   21.602275]  kernel_init_freeable+0x20c/0x3e0
> > [   21.602664]  kernel_init+0x2c/0x1f8
> > [   21.602979]  ret_from_fork+0x10/0x20
>
> This doesn't make a lot of sense. "remote-endpoint" shouldn't be
> related to anything to do with psci cpuidle. I'm guessing something
> else is failing much earlier in boot that's indirectly causing this
> somehow? Can you please take a look at what's failing earlier and let
> us know? Or see what driver probe is failing up to this point but used
> to work in the good case.

I'm pretty new to this, "just starting". I'm not sure how to do that,
since the kernel doesn't really "move forward". I will verify if
a8037ceb8964 fixes it or not and get back by the end of this week.

> Also, where is the dts file that corresponds to this board in upstream? Is it
> arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts

Yes.

> >
> > > Also, can you give the output of <debugfs>/devices_deferred for the
> > > good vs bad case?
> >
> > I can't provide you with requested output from the bad case, since the
> > kernel never moves past this to an initramfs rescue shell, but following
> > is the output from v6.8.1 (**with aforementioned patch reverted**).
> >
> > # cat /sys/kernel/debug/devices_deferred
> > fc400000.usb    platform: wait for supplier /phy@fed90000/usb3-port
> > 1-0022  typec_fusb302: cannot register tcpm port
> > fc000000.usb    platform: wait for supplier /phy@fed80000/usb3-port
> >
> > It seems that v6.8.2 works _without needing to revert the patch_. I will
> > have to look into this sometime this week but it seems like
> > a8037ceb8964 (arm64: dts: rockchip: drop rockchip,trcm-sync-tx-only from rk3588 i2s)
> > seems to be the one that fixed the root issue. I will have to test it
> > sometime later this week.
>
> Ok, once you find the patch that fixes things, let me know too.

Will do!

 -- Pratham Patel


^ permalink raw reply

* Re: [PATCH] ARM: dts: imx7s-warp: Pass OV2680 link-frequencies
From: Fabio Estevam @ 2024-04-03  0:56 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Fabio Estevam, linux-arm-kernel, sakari.ailus,
	stable, hdegoede, devicetree, shawnguo, conor+dt,
	krzysztof.kozlowski+dt
In-Reply-To: <ZgyoX55PJD0LDSPf@dragon>

Hi Shawn,

On Tue, Apr 2, 2024 at 9:53 PM Shawn Guo <shawnguo2@yeah.net> wrote:

> So it sounds like that the bindings doc needs an update to include
> 'link-frequencies'?

Correct. I have already submitted the binding patch:
https://lore.kernel.org/linux-devicetree/20240402174028.205434-2-festevam@gmail.com/

^ permalink raw reply

* Re: [PATCH] ARM: dts: imx7s-warp: Pass OV2680 link-frequencies
From: Shawn Guo @ 2024-04-03  0:52 UTC (permalink / raw)
  To: Rob Herring
  Cc: Fabio Estevam, Fabio Estevam, linux-arm-kernel, sakari.ailus,
	stable, hdegoede, devicetree, shawnguo, conor+dt,
	krzysztof.kozlowski+dt
In-Reply-To: <171165955888.338117.15736314486472326706.robh@kernel.org>

On Thu, Mar 28, 2024 at 04:01:02PM -0500, Rob Herring wrote:
> 
> On Thu, 28 Mar 2024 12:19:54 -0300, Fabio Estevam wrote:
> > From: Fabio Estevam <festevam@denx.de>
> > 
> > Since commit 63b0cd30b78e ("media: ov2680: Add bus-cfg / endpoint
> > property verification") the ov2680 no longer probes on a imx7s-warp7:
> > 
> > ov2680 1-0036: error -EINVAL: supported link freq 330000000 not found
> > ov2680 1-0036: probe with driver ov2680 failed with error -22
> > 
> > Fix it by passing the required 'link-frequencies' property as
> > recommended by:
> > 
> > https://www.kernel.org/doc/html/v6.9-rc1/driver-api/media/camera-sensor.html#handling-clocks
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 63b0cd30b78e ("media: ov2680: Add bus-cfg / endpoint property verification")
> > Signed-off-by: Fabio Estevam <festevam@denx.de>
> > ---
> >  arch/arm/boot/dts/nxp/imx/imx7s-warp.dts | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> 
> 
> My bot found new DTB warnings on the .dts files added or changed in this
> series.
> 
> Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
> are fixed by another series. Ultimately, it is up to the platform
> maintainer whether these warnings are acceptable or not. No need to reply
> unless the platform maintainer has comments.
> 
> If you already ran DT checks and didn't see these error(s), then
> make sure dt-schema is up to date:
> 
>   pip3 install dtschema --upgrade
> 
> 
> New warnings running 'make CHECK_DTBS=y nxp/imx/imx7s-warp.dtb' for 20240328151954.2517368-1-festevam@gmail.com:
> 
> arch/arm/boot/dts/nxp/imx/imx7s-warp.dtb: camera@36: port:endpoint: Unevaluated properties are not allowed ('clock-lanes', 'data-lanes', 'link-frequencies' were unexpected)
> 	from schema $id: http://devicetree.org/schemas/media/i2c/ovti,ov2680.yaml#

So it sounds like that the bindings doc needs an update to include
'link-frequencies'?

Shawn


^ permalink raw reply

* Re: [PATCH] ARM: dts: imx7s-warp: Pass OV2680 link-frequencies
From: Shawn Guo @ 2024-04-03  0:51 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: shawnguo, sakari.ailus, hdegoede, robh, krzysztof.kozlowski+dt,
	conor+dt, linux-arm-kernel, devicetree, Fabio Estevam, stable
In-Reply-To: <20240328151954.2517368-1-festevam@gmail.com>

On Thu, Mar 28, 2024 at 12:19:54PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@denx.de>
> 
> Since commit 63b0cd30b78e ("media: ov2680: Add bus-cfg / endpoint
> property verification") the ov2680 no longer probes on a imx7s-warp7:
> 
> ov2680 1-0036: error -EINVAL: supported link freq 330000000 not found
> ov2680 1-0036: probe with driver ov2680 failed with error -22
> 
> Fix it by passing the required 'link-frequencies' property as
> recommended by:
> 
> https://www.kernel.org/doc/html/v6.9-rc1/driver-api/media/camera-sensor.html#handling-clocks
> 
> Cc: stable@vger.kernel.org
> Fixes: 63b0cd30b78e ("media: ov2680: Add bus-cfg / endpoint property verification") 
> Signed-off-by: Fabio Estevam <festevam@denx.de>

Applied, thanks!


^ permalink raw reply

* Re: [PATCH v3 1/4] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
From: Dmitry Baryshkov @ 2024-04-03  0:51 UTC (permalink / raw)
  To: Marijn Suijten
  Cc: Krzysztof Kozlowski, Sumit Semwal, Caleb Connolly, Neil Armstrong,
	Jessica Zhang, Sam Ravnborg, David Airlie, Daniel Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, dri-devel, devicetree,
	linux-kernel, linux-arm-msm, Vinod Koul, Caleb Connolly
In-Reply-To: <t3cx5qxiteer27vsvysizbrxkbamxgrcbn2oafisodjopwas5z@nxlasb4rlnml>

On Tue, Apr 02, 2024 at 10:59:11PM +0200, Marijn Suijten wrote:
> On 2024-04-02 10:23:22, Dmitry Baryshkov wrote:
> > On Tue, 2 Apr 2024 at 09:31, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > >
> > > On 02/04/2024 01:51, Dmitry Baryshkov wrote:
> > > > From: Sumit Semwal <sumit.semwal@linaro.org>
> > > >
> > > > LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel present on Google Pixel 3
> > > > phones.
> > > >
> > > > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > > > Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> > > > [caleb: convert to yaml]
> > > > Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > ---
> > >
> > > Tags missing.
> > >
> > > `b4 trailers -u`
> > 
> > Excuse me, I keep on forgetting it.
> 
> Does a similar thing exist for adding Cc: tags for all reviewers/replyers to an
> earlier version, even if said reviewer didn't yet provide R-b/A-b or other tags?
> 
> I'd like to have the next revisions in my inbox as well after leaving
> comments :)

Unfortunately I don't know such option.

> 
> Thanks! - Marijn

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH v8 5/5] arm64: dts: imx8qxp: add asrc[0,1], esai0, spdif0 and sai[4,5]
From: Shawn Guo @ 2024-04-03  0:46 UTC (permalink / raw)
  To: Frank Li
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-sound, devicetree, imx,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20240328-asrc_8qxp-v8-5-801cd6bb5be2@nxp.com>

On Thu, Mar 28, 2024 at 10:51:37AM -0400, Frank Li wrote:
> Add asrc[0,1], esai0, spdif0, sai[4,5] and related lpcg node for
> imx8 audio subsystem.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Applied, thanks!


^ permalink raw reply

* Re: [PATCH v8 4/5] arm64: dts: imx8: fix audio lpcg index
From: Shawn Guo @ 2024-04-03  0:46 UTC (permalink / raw)
  To: Frank Li
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Shengjiu Wang, linux-sound, devicetree, imx,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20240328-asrc_8qxp-v8-4-801cd6bb5be2@nxp.com>

On Thu, Mar 28, 2024 at 10:51:36AM -0400, Frank Li wrote:
> lpcg cell0 should be clock's 'indices' instead of 'index'.
> 
> imx_lpcg_of_clk_src_get(struct of_phandle_args *clkspec, void *data)
> {
>         struct clk_hw_onecell_data *hw_data = data;
>         unsigned int idx = clkspec->args[0] / 4;
> 
> 	....
> }
> 
> <@sai0_lpcg 1> will be the same as <@sai_lpcg 0>.
> 
> Replace 0 with IMX_LPCG_CLK_0 and replace 1 with IMX_LPCG_CLK_4.
> 
> It can work at iMX8QXP because IMX_LPCG_CLK_4 is ipg clock, which already
> enabled. But for iMX8QM IMX_LPCG_CLK_4 is mclk, which trigger issue.
> 
> Fixes: 0a9279e9ae88 ("arm64: dts: imx8qxp: Add audio SAI nodes")
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Applied, thanks!


^ permalink raw reply

* Re: Fixing the devicetree of Rock 5 Model B (and possibly others)
From: Saravana Kannan @ 2024-04-03  0:46 UTC (permalink / raw)
  To: Pratham Patel
  Cc: Dragan Simic, sebastian.reichel, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel, regressions, stable
In-Reply-To: <D0A122WK7CB9.33B2TP6UCMJBJ@thefossguy.com>

On Tue, Apr 2, 2024 at 4:32 PM Pratham Patel
<prathampatel@thefossguy.com> wrote:
>
> On Tue Apr 2, 2024 at 4:54 AM IST, Saravana Kannan wrote:
> > On Sat, Mar 23, 2024 at 10:10 AM Dragan Simic <dsimic@manjaro.org> wrote:
> > >
> > > Hello Pratham,
> > >
> > > On 2024-03-23 18:02, Pratham Patel wrote:
> > > > I looked at the patch and tried several things, neither resulted in
> > > > anything that would point me to the core issue. Then I tried this:
> > >
> > > Could you, please, clarify a bit what's the actual issue you're
> > > experiencing on your Rock 5B?
> >
> > Pratham, can you reply to this please? I don't really understand what
> > your issue is for me to be able to help.
>
> Hi,
>
> I apologize for not replying. Somehow, I did not notice the reply from
> Dragan. :(
>
> Since this patch was applied, an issue in the Rock 5B's DT has been
> unearthed which now results in the kernel being unable to boot properly.
>
> Following is the relevant call trace from the UART capture:
>
> [   21.595068] Call trace:
> [   21.595288]  smp_call_function_many_cond+0x174/0x5f8
> [   21.595728]  on_each_cpu_cond_mask+0x2c/0x40
> [   21.596109]  cpuidle_register_driver+0x294/0x318
> [   21.596524]  cpuidle_register+0x24/0x100
> [   21.596875]  psci_cpuidle_probe+0x2e4/0x490
> [   21.597247]  platform_probe+0x70/0xd0
> [   21.597575]  really_probe+0x18c/0x3d8
> [   21.597905]  __driver_probe_device+0x84/0x180
> [   21.598294]  driver_probe_device+0x44/0x120
> [   21.598669]  __device_attach_driver+0xc4/0x168
> [   21.599063]  bus_for_each_drv+0x8c/0xf0
> [   21.599408]  __device_attach+0xa4/0x1c0
> [   21.599748]  device_initial_probe+0x1c/0x30
> [   21.600118]  bus_probe_device+0xb4/0xc0
> [   21.600462]  device_add+0x68c/0x888
> [   21.600775]  platform_device_add+0x19c/0x270
> [   21.601154]  platform_device_register_full+0xdc/0x178
> [   21.601602]  psci_idle_init+0xa0/0xc8
> [   21.601934]  do_one_initcall+0x60/0x290
> [   21.602275]  kernel_init_freeable+0x20c/0x3e0
> [   21.602664]  kernel_init+0x2c/0x1f8
> [   21.602979]  ret_from_fork+0x10/0x20

This doesn't make a lot of sense. "remote-endpoint" shouldn't be
related to anything to do with psci cpuidle. I'm guessing something
else is failing much earlier in boot that's indirectly causing this
somehow? Can you please take a look at what's failing earlier and let
us know? Or see what driver probe is failing up to this point but used
to work in the good case.

Also, where is the dts file that corresponds to this board in upstream? Is it
arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts

>
> > Also, can you give the output of <debugfs>/devices_deferred for the
> > good vs bad case?
>
> I can't provide you with requested output from the bad case, since the
> kernel never moves past this to an initramfs rescue shell, but following
> is the output from v6.8.1 (**with aforementioned patch reverted**).
>
> # cat /sys/kernel/debug/devices_deferred
> fc400000.usb    platform: wait for supplier /phy@fed90000/usb3-port
> 1-0022  typec_fusb302: cannot register tcpm port
> fc000000.usb    platform: wait for supplier /phy@fed80000/usb3-port
>
> It seems that v6.8.2 works _without needing to revert the patch_. I will
> have to look into this sometime this week but it seems like
> a8037ceb8964 (arm64: dts: rockchip: drop rockchip,trcm-sync-tx-only from rk3588 i2s)
> seems to be the one that fixed the root issue. I will have to test it
> sometime later this week.

Ok, once you find the patch that fixes things, let me know too.

-Saravana

^ permalink raw reply

* Re: [PATCH] arm64: dts: lx2160a: add pinmux and i2c gpio to support bus recovery
From: Shawn Guo @ 2024-04-03  0:43 UTC (permalink / raw)
  To: carlos.song
  Cc: shawnguo, robh, krzysztof.kozlowski+dt, conor+dt, frank.li,
	linux-arm-kernel, devicetree, linux-kernel
In-Reply-To: <20240328061419.1956329-1-carlos.song@nxp.com>

On Thu, Mar 28, 2024 at 02:14:19PM +0800, carlos.song@nxp.com wrote:
> From: Carlos Song <carlos.song@nxp.com>
> 
> I2C bus recovery need a pinmux and gpio. So i2c driver can switch
> gpio mode to toggle scl to recovery bus.
> 
> Add pinctrl-single node to every i2c bus on fsl-ls2160 layerscape
> platform.
> 
> Signed-off-by: Carlos Song <carlos.song@nxp.com>
> Reviewed-by: Frank Li <frank.li@nxp.com>

Applied, thanks!


^ permalink raw reply

* Re: [PATCH 3/3] drm: panel: Add LG sw43408 panel driver
From: Dmitry Baryshkov @ 2024-04-03  0:36 UTC (permalink / raw)
  To: Marijn Suijten
  Cc: Sumit Semwal, Caleb Connolly, Neil Armstrong, Jessica Zhang,
	Sam Ravnborg, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, dri-devel, devicetree,
	linux-kernel, linux-arm-msm, Vinod Koul, Caleb Connolly
In-Reply-To: <rdvcvkofafpdovldlibvobsxlk7nt3cc4kysm4aiafux7bmai6@nsqs7ofgr3ol>

On Tue, 2 Apr 2024 at 23:57, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> On 2024-04-01 22:11:48, Dmitry Baryshkov wrote:
> > On Mon, 1 Apr 2024 at 13:29, Marijn Suijten
> > <marijn.suijten@somainline.org> wrote:
> > >
> > > On 2024-03-30 16:37:08, Dmitry Baryshkov wrote:
> > > > On Sat, 30 Mar 2024 at 12:27, Marijn Suijten
> > > > <marijn.suijten@somainline.org> wrote:
> > > > >
> > > > > On 2024-03-30 05:59:30, Dmitry Baryshkov wrote:
> > > > > > From: Sumit Semwal <sumit.semwal@linaro.org>
> > > > > >
> > > > > > LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel, used in some Pixel3
> > > > > > phones.
> > > > > >
> > > > > > Whatever init sequence we have for this panel isn't capable of
> > > > > > initialising it completely, toggling the reset gpio ever causes the
> > > > > > panel to die. Until this is resolved we avoid resetting the panel. The
> > > > >
> > > > > Are you sure it is avoided?  This patch seems to be toggling reset_gpio in
> > > > > sw43408_prepare()?
> > > > >
> > > > > > disable/unprepare functions only put the panel to sleep mode and
> > > > > > disable the backlight.
> > > > > >
> > > > > > Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> > > > > > [vinod: Add DSC support]
> > > > > > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > > > > > [caleb: cleanup and support turning off the panel]
> > > > > > Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> > > > > > [DB: partially rewrote the driver and fixed DSC programming]
> > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > > > ---
> > > > > >  MAINTAINERS                              |   8 +
> > > > > >  drivers/gpu/drm/panel/Kconfig            |  11 ++
> > > > > >  drivers/gpu/drm/panel/Makefile           |   1 +
> > > > > >  drivers/gpu/drm/panel/panel-lg-sw43408.c | 322 +++++++++++++++++++++++++++++++
> > > > > >  4 files changed, 342 insertions(+)
> > > > > >
> > > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > > index 4b511a55101c..f4cf7ee97376 100644
> > > > > > --- a/MAINTAINERS
> > > > > > +++ b/MAINTAINERS
> > > > > > @@ -6755,6 +6755,14 @@ S:     Maintained
> > > > > >  F:   Documentation/devicetree/bindings/display/panel/jadard,jd9365da-h3.yaml
> > > > > >  F:   drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
> > > > > >
> > > > > > +DRM DRIVER FOR LG SW43408 PANELS
> > > > > > +M:   Sumit Semwal <sumit.semwal@linaro.org>
> > > > > > +M:   Caleb Connolly <caleb.connolly@linaro.org>
> > > > > > +S:   Maintained
> > > > > > +T:   git git://anongit.freedesktop.org/drm/drm-misc
> > > > > > +F:   Documentation/devicetree/bindings/display/panel/lg,sw43408.yaml
> > > > > > +F:   drivers/gpu/drm/panel/panel-lg-sw43408.c
> > > > > > +
> > > > > >  DRM DRIVER FOR LOGICVC DISPLAY CONTROLLER
> > > > > >  M:   Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > >  S:   Supported
> > > > > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > > > > > index d037b3b8b999..f94c702735cb 100644
> > > > > > --- a/drivers/gpu/drm/panel/Kconfig
> > > > > > +++ b/drivers/gpu/drm/panel/Kconfig
> > > > > > @@ -335,6 +335,17 @@ config DRM_PANEL_LG_LG4573
> > > > > >         Say Y here if you want to enable support for LG4573 RGB panel.
> > > > > >         To compile this driver as a module, choose M here.
> > > > > >
> > > > > > +config DRM_PANEL_LG_SW43408
> > > > > > +     tristate "LG SW43408 panel"
> > > > > > +     depends on OF
> > > > > > +     depends on DRM_MIPI_DSI
> > > > > > +     depends on BACKLIGHT_CLASS_DEVICE
> > > > > > +     help
> > > > > > +       Say Y here if you want to enable support for LG sw43408 panel.
> > > > > > +       The panel has a 1080x2160 resolution and uses
> > > > > > +       24 bit RGB per pixel. It provides a MIPI DSI interface to
> > > > > > +       the host and has a built-in LED backlight.
> > > > > > +
> > > > > >  config DRM_PANEL_MAGNACHIP_D53E6EA8966
> > > > > >       tristate "Magnachip D53E6EA8966 DSI panel"
> > > > > >       depends on OF && SPI
> > > > > > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > > > > > index f156d7fa0bcc..a75687d13caf 100644
> > > > > > --- a/drivers/gpu/drm/panel/Makefile
> > > > > > +++ b/drivers/gpu/drm/panel/Makefile
> > > > > > @@ -34,6 +34,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
> > > > > >  obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
> > > > > >  obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
> > > > > >  obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> > > > > > +obj-$(CONFIG_DRM_PANEL_LG_SW43408) += panel-lg-sw43408.o
> > > > > >  obj-$(CONFIG_DRM_PANEL_MAGNACHIP_D53E6EA8966) += panel-magnachip-d53e6ea8966.o
> > > > > >  obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
> > > > > >  obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3051D) += panel-newvision-nv3051d.o
> > > > > > diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> > > > > > new file mode 100644
> > > > > > index 000000000000..365d25e14d54
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> > > > > > @@ -0,0 +1,322 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > +/*
> > > > > > + * Copyright (C) 2019-2024 Linaro Ltd
> > > > > > + * Author: Sumit Semwal <sumit.semwal@linaro.org>
> > > > > > + *    Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > > > + */
> > > > > > +
> > > > > > +#include <linux/backlight.h>
> > > > > > +#include <linux/delay.h>
> > > > > > +#include <linux/gpio/consumer.h>
> > > > > > +#include <linux/module.h>
> > > > > > +#include <linux/of.h>
> > > > > > +#include <linux/regulator/consumer.h>
> > > > > > +
> > > > > > +#include <video/mipi_display.h>
> > > > > > +
> > > > > > +#include <drm/drm_mipi_dsi.h>
> > > > > > +#include <drm/drm_panel.h>
> > > > > > +#include <drm/drm_probe_helper.h>
> > > > > > +#include <drm/display/drm_dsc.h>
> > > > > > +#include <drm/display/drm_dsc_helper.h>
> > > > > > +
> > > > > > +#define NUM_SUPPLIES 2
> > > > > > +
> > > > > > +struct sw43408_panel {
> > > > > > +     struct drm_panel base;
> > > > > > +     struct mipi_dsi_device *link;
> > > > > > +
> > > > > > +     const struct drm_display_mode *mode;
> > > > > > +
> > > > > > +     struct regulator_bulk_data supplies[NUM_SUPPLIES];
> > > > > > +
> > > > > > +     struct gpio_desc *reset_gpio;
> > > > > > +};
> > > > > > +
> > > > > > +static inline struct sw43408_panel *to_panel_info(struct drm_panel *panel)
> > > > > > +{
> > > > > > +     return container_of(panel, struct sw43408_panel, base);
> > > > > > +}
> > > > > > +
> > > > > > +static int sw43408_unprepare(struct drm_panel *panel)
> > > > > > +{
> > > > > > +     struct sw43408_panel *ctx = to_panel_info(panel);
> > > > > > +     int ret;
> > > > > > +
> > > > > > +     ret = mipi_dsi_dcs_set_display_off(ctx->link);
> > > > > > +     if (ret < 0)
> > > > > > +             dev_err(panel->dev, "set_display_off cmd failed ret = %d\n", ret);
> > > > > > +
> > > > > > +     ret = mipi_dsi_dcs_enter_sleep_mode(ctx->link);
> > > > > > +     if (ret < 0)
> > > > > > +             dev_err(panel->dev, "enter_sleep cmd failed ret = %d\n", ret);
> > > > > > +
> > > > > > +     msleep(100);
> > > > > > +
> > > > > > +     gpiod_set_value(ctx->reset_gpio, 1);
> > > > > > +
> > > > > > +     return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > > > > > +}
> > > > > > +
> > > > > > +static int sw43408_program(struct drm_panel *panel)
> > > > > > +{
> > > > > > +     struct sw43408_panel *ctx = to_panel_info(panel);
> > > > > > +     struct drm_dsc_picture_parameter_set pps;
> > > > > > +     u8 dsc_en = 0x11;
> > > > >
> > > > > Yeah, this is completely strange. Bit 0, 0x1, is to enable DSC which is
> > > > > normal. 0x10 however, which is bit 4, selects PPS table 2.  Do you ever set
> > > > > pps_identifier in struct drm_dsc_picture_parameter_set to 2?  Or is the table
> > > > > that you send below bogus and/or not used?  Maybe the Driver IC on the other
> > > > > end of the DSI link has a default PPS table with identifier 2 that works out of
> > > > > the box?
> > > >
> > > > Note, MIPI standard also requires two bytes argument. I suspect that
> > > > LG didn't fully follow the standard here.
> > >
> > > Have you read this command from downstream DTS, or have you tried sending 2
> > > bytes and seen the panel breaking?  The second byte is marked as reserved and
> > > should be equal to 0; if the Driver IC is okay with sending either 1 or 2 bytes
> > > I'd strive to stick with the defined length of 2 bytes for this DCS.
> > >
> > > Have you played around with the PPS table?  What if you change
> > > drm_dsc_picture_paremeter_set::pps_identifier to the second table, will the
> > > panel stop working as expected again?  This could indicate that the PPS that is
> > > sent is incorrect (even though the information in the original DSC config was
> > > enough to set up the DPU and DSI correctly).
> > >
> > > According to the DSI spec it is allowed to have a pre-stored/pre-programmed
> > > PPS table, which could be used here making the current call to
> > > mipi_dsi_picture_parameter_set() useless and "confusing"?
> >
> > Ok, some short summary of my tests.
> >
> > Skipping PPS doesn't work at all, so there is no default.
> >
> > Adding a second zero byte doesn't seem to change anything. Dropping
> > the 0x1 bit ('enable') doesn't seem to change anything.
> >
> > If I send COMPRESSION_MODE before sending the PPS, various combinations work.
> > If I send COMPRESSION_MODE after sending the PPS, the follow combos work:
> >
> > pps_identifier = 0x0, COMPRESSION_MODE = 0x11
> > pps_identifier = 0x1, COMPRESSION_MODE = 0x21
>
> Thanks, this must really be an off-by-one table identifier.  I presume you've
> tested pps_identifier=0x2 with COMPRESSION_MODE=0x31, and that there are only 2
> tables and not 3 or 4?

I was not able to get either pps_identifier = 0x2 or
COMPRESSION_MODE=0x31 to work.

>
> From this we can also assume that sending a new PPS will automatically switch
> the compression mode to the pps_identifier in that PPS, COMPRESSION_MODE doesn't
> seem to affect it when sent too early.

No. Omitting COMPRESSION_MODE packet breaks the display. Actually,
this was one of the issues: we were sending an incorrect packet. So
both are required.

>
> > > > Basically that's the reason why I went for the _raw function instead
> > > > of adding PPS and codec arguments to the existing function.
> > > >
> > > > >
> > > > > > +     mipi_dsi_dcs_write_seq(ctx->link, MIPI_DCS_SET_GAMMA_CURVE, 0x02);
> > > > > > +
> > > > > > +     mipi_dsi_dcs_set_tear_on(ctx->link, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> > > > > > +
> > > > > > +     mipi_dsi_dcs_write_seq(ctx->link, 0x53, 0x0c, 0x30);
> > > > > > +     mipi_dsi_dcs_write_seq(ctx->link, 0x55, 0x00, 0x70, 0xdf, 0x00, 0x70, 0xdf);
> > > > > > +     mipi_dsi_dcs_write_seq(ctx->link, 0xf7, 0x01, 0x49, 0x0c);
> > > > > > +
> > > > > > +     mipi_dsi_dcs_exit_sleep_mode(ctx->link);
> > > > > > +
> > > > > > +     msleep(135);
> > > > > > +
> > > > > > +     mipi_dsi_compression_mode_raw(ctx->link, &dsc_en, 1);
> > > > >
> > > > > Even though I think we should change this function to describe the known
> > > > > bit layout of command 0x7 per the VESA DSI spec, for now replace 1 with
> > > > > sizeof(dsc_en)?
> > > >
> > > > If dsc_en were an array, it would have been a proper thing. Maybe I
> > > > should change it to the array to remove confusion.
> > >
> > > It should work even with a single byte, just to clarify to readers that the 3rd
> > > argument is the byte-size of the input.
> > >
> > > > > > +     mipi_dsi_dcs_write_seq(ctx->link, 0xb0, 0xac);
> > > > > > +     mipi_dsi_dcs_write_seq(ctx->link, 0xe5,
> > > > > > +                            0x00, 0x3a, 0x00, 0x3a, 0x00, 0x0e, 0x10);
> > > > > > +     mipi_dsi_dcs_write_seq(ctx->link, 0xb5,
> > > > > > +                            0x75, 0x60, 0x2d, 0x5d, 0x80, 0x00, 0x0a, 0x0b,
> > > > > > +                            0x00, 0x05, 0x0b, 0x00, 0x80, 0x0d, 0x0e, 0x40,
> > > > > > +                            0x00, 0x0c, 0x00, 0x16, 0x00, 0xb8, 0x00, 0x80,
> > > > > > +                            0x0d, 0x0e, 0x40, 0x00, 0x0c, 0x00, 0x16, 0x00,
> > > > > > +                            0xb8, 0x00, 0x81, 0x00, 0x03, 0x03, 0x03, 0x01,
> > > > > > +                            0x01);
> > > > > > +     msleep(85);
> > > > > > +     mipi_dsi_dcs_write_seq(ctx->link, 0xcd,
> > > > > > +                            0x00, 0x00, 0x00, 0x19, 0x19, 0x19, 0x19, 0x19,
> > > > > > +                            0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x19,
> > > > > > +                            0x16, 0x16);
> > > > > > +     mipi_dsi_dcs_write_seq(ctx->link, 0xcb, 0x80, 0x5c, 0x07, 0x03, 0x28);
> > > > > > +     mipi_dsi_dcs_write_seq(ctx->link, 0xc0, 0x02, 0x02, 0x0f);
> > > > > > +     mipi_dsi_dcs_write_seq(ctx->link, 0x55, 0x04, 0x61, 0xdb, 0x04, 0x70, 0xdb);
> > > > > > +     mipi_dsi_dcs_write_seq(ctx->link, 0xb0, 0xca);
> > > > > > +
> > > > > > +     mipi_dsi_dcs_set_display_on(ctx->link);
> > > > >
> > > > > Any specific reason to not have the (un)blanking sequence in the enable/disable
> > > > > callbacks and leaving display configuration in (un)prepare?
> > > >
> > > > We are back to the question on when it's fine to send the commands. I
> > > > think the current agreement is to send everything in the
> > > > prepare/unprepare, because of some strange hosts.
> > >
> > > For my panel drivers I'm sticking with having `post-on` commands (from
> > > downstream) in `enable/disable`, which is typically only `set_display_on`.  In
> > > hopes of proposing a `prepare_atomic()` some time to allow mode selection.
> > >
> > > In a short test on recent -next I am once again allowed to send DSI commands in
> > > both .disable and .unprepare, making both functions a "clean" inverse of .enable
> > > and .prepare respectively.
> >
> > The world isn't limited to the MSM hosts.
>
> If I'm not mistaken this was an ordering issue in the drm_bridge implementation.

No. I think sunxi didn't support sending DSI commands after starting
the video stream.

>
> But you are right that some hosts might not be all too happy with sending
> commands (like unblanking?) after the cmd/video stream started, and before the
> stream stops.  Which, as far as I know, are what .enable and .disable do.  On
> the other hand, I was under the impression that this split mainly existed to do
> all the heavy/required lifting up-front, and only unblank when there's a video
> signal to combat any possible observed corruption?

In the ideal world that would be true. But we are not living in the
ideal world. I still have hopes to get back to the idea of reworling
this part of the DSI framework.

> In the end I'm just curious if there's a specific reason - that I need
> to take into account when resending all my panel patches - to /not/
> use .enable/.disable?

I have bookmarked this email as a main reference for the topic:
https://lore.kernel.org/dri-devel/CAPY8ntBrhYAmsraDqJGuTrSL6VjGXBAMVoN7xweV7E4qZv+v3Q@mail.gmail.com/

>
> - Marijn
>
> > > > > > +     msleep(50);
> > > > > > +
> > > > > > +     ctx->link->mode_flags &= ~MIPI_DSI_MODE_LPM;
> > > > > > +
> > > > > > +     drm_dsc_pps_payload_pack(&pps, ctx->link->dsc);
> > > > > > +     mipi_dsi_picture_parameter_set(ctx->link, &pps);
> > > > >
> > > > > I'm always surprised why this is sent _after_ turning the display on (unblanking
> > > > > it).  Wouldn't that cause unnecessary corruption?
> > > >
> > > > No idea. I followed the dowsntream command sequences here. Most likely
> > > > the panel is not fully on until it receives the full frame to be
> > > > displayed.
> > >
> > > According to the DSI spec a PPS update is allowed to happen every frame, and
> > > (for cmdmode panels) will take effect after the next TE trigger.  Unsure if a TE
> > > event happens before the first frame, otherwise this may start taking effect
> > > on the second frame onwards only.
> > >
> > > If there's no corruption on the first frame there might be a pre-programmed PPS
> > > table in slot 2, supporting the theory above.
> >
> >
> >
> > --
> > With best wishes
> > Dmitry



--
With best wishes
Dmitry

^ permalink raw reply

* [PATCH v8 4/4] ASoc: dt-bindings: PCM6240: Add initial DT binding
From: Shenghao Ding @ 2024-04-03  0:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-sound, devicetree, perex, tiwai, 13916275206, mohit.chawla,
	soyer, jkhuang3, tiwai, pdjuandi, manisha.agrawal, aviel,
	hnagalla, praneeth, Baojun.Xu, Shenghao Ding, Rob Herring
In-Reply-To: <20240403003159.389-1-shenghao-ding@ti.com>

PCM6240 family chips are popular among audio customers, in spite of only a
portion of the functionality of codec, such as ADC or DAC, and so on, for
different Specifications, range from Personal Electric to Automotive
Electric, even some professional fields. Yet their audio performance is far
superior to the codec's, and cost is lower than codec, and much easier to
program than codec.

Reviewed-by: Rob Herring <robh@kernel.org>

Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>

---
v8:
 - use some reasonable format of changelog.
 - Link to v5: https://lore.kernel.org/all/20240221051501.627-4-shenghao-ding@ti.com/
---
 .../devicetree/bindings/sound/ti,pcm6240.yaml | 177 ++++++++++++++++++
 1 file changed, 177 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/ti,pcm6240.yaml

diff --git a/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml b/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml
new file mode 100644
index 000000000000..dd5b08e3d7a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml
@@ -0,0 +1,177 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 - 2024 Texas Instruments Incorporated
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/ti,pcm6240.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments PCM6240 Family Audio ADC/DAC
+
+maintainers:
+  - Shenghao Ding <shenghao-ding@ti.com>
+
+description: |
+  The PCM6240 Family is a big family of Audio ADC/DAC for
+  different Specifications, range from Personal Electric
+  to Automotive Electric, even some professional fields.
+
+  Specifications about the audio chip can be found at:
+    https://www.ti.com/lit/gpn/tlv320adc3120
+    https://www.ti.com/lit/gpn/tlv320adc5120
+    https://www.ti.com/lit/gpn/tlv320adc6120
+    https://www.ti.com/lit/gpn/dix4192
+    https://www.ti.com/lit/gpn/pcm1690
+    https://www.ti.com/lit/gpn/pcm3120-q1
+    https://www.ti.com/lit/gpn/pcm3140-q1
+    https://www.ti.com/lit/gpn/pcm5120-q1
+    https://www.ti.com/lit/gpn/pcm6120-q1
+    https://www.ti.com/lit/gpn/pcm6260-q1
+    https://www.ti.com/lit/gpn/pcm9211
+    https://www.ti.com/lit/gpn/pcmd3140
+    https://www.ti.com/lit/gpn/pcmd3180
+    https://www.ti.com/lit/gpn/taa5212
+    https://www.ti.com/lit/gpn/tad5212
+
+properties:
+  compatible:
+    description: |
+      ti,adc3120: Stereo-channel, 768-kHz, Burr-Brown™ audio analog-to-
+      digital converter (ADC) with 106-dB SNR.
+
+      ti,adc5120: 2-Channel, 768-kHz, Burr-Brown™ Audio ADC with 120-dB SNR.
+
+      ti,adc6120: Stereo-channel, 768-kHz, Burr-Brown™ audio analog-to-
+      digital converter (ADC) with 123-dB SNR.
+
+      ti,dix4192: 216-kHz digital audio converter with Quad-Channel In
+      and One-Channel Out.
+
+      ti,pcm1690: Automotive Catalog 113dB SNR 8-Channel Audio DAC with
+      Differential Outputs.
+
+      ti,pcm3120: Automotive, stereo, 106-dB SNR, 768-kHz, low-power
+      software-controlled audio ADC.
+
+      ti,pcm3140: Automotive, Quad-Channel, 768-kHz, Burr-Brown™ Audio ADC
+      with 106-dB SNR.
+
+      ti,pcm5120: Automotive, stereo, 120-dB SNR, 768-kHz, low-power
+      software-controlled audio ADC.
+
+      ti,pcm5140: Automotive, Quad-Channel, 768-kHz, Burr-Brown™ Audio ADC
+      with 120-dB SNR.
+
+      ti,pcm6120: Automotive, stereo, 123-dB SNR, 768-kHz, low-power
+      software-controlled audio ADC.
+
+      ti,pcm6140: Automotive, Quad-Channel, 768-kHz, Burr-Brown™ Audio ADC
+      with 123-dB SNR.
+
+      ti,pcm6240: Automotive 4-ch audio ADC with integrated programmable mic
+      bias, boost and input diagnostics.
+
+      ti,pcm6260: Automotive 6-ch audio ADC with integrated programmable mic
+      bias, boost and input diagnostics.
+
+      ti,pcm9211: 216-kHz digital audio converter With Stereo ADC and
+      Routing.
+
+      ti,pcmd3140: Four-channel PDM-input to TDM or I2S output converter.
+
+      ti,pcmd3180: Eight-channel pulse-density-modulation input to TDM or
+      I2S output converter.
+
+      ti,taa5212: Low-power high-performance stereo audio ADC with 118-dB
+      dynamic range.
+
+      ti,tad5212: Low-power stereo audio DAC with 120-dB dynamic range.
+    oneOf:
+      - items:
+          - enum:
+              - ti,adc3120
+              - ti,adc5120
+              - ti,pcm3120
+              - ti,pcm5120
+              - ti,pcm6120
+          - const: ti,adc6120
+      - items:
+          - enum:
+              - ti,pcmd512x
+              - ti,pcm9211
+              - ti,taa5212
+              - ti,tad5212
+          - const: ti,adc6120
+      - items:
+          - enum:
+              - ti,pcm3140
+              - ti,pcm5140
+              - ti,dix4192
+              - ti,pcm6140
+              - ti,pcm6260
+          - const: ti,pcm6240
+      - items:
+          - enum:
+              - ti,pcmd3140
+              - ti,pcmd3180
+              - ti,pcm1690
+              - ti,taa5412
+              - ti,tad5412
+          - const: ti,pcm6240
+      - enum:
+          - ti,adc6120
+          - ti,pcm6240
+
+  reg:
+    description:
+      I2C address, in multiple pcmdevices case, all the i2c address
+      aggregate as one Audio Device to support multiple audio slots.
+    minItems: 1
+    maxItems: 4
+
+  reset-gpios:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+    description:
+      Invalid only for ti,pcm1690 because of no INT pin.
+
+  '#sound-dai-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: dai-common.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - ti,pcm1690
+    then:
+      properties:
+        interrupts: false
+
+additionalProperties: false
+
+examples:
+  - |
+   #include <dt-bindings/gpio/gpio.h>
+   i2c {
+     /* example for two devices with interrupt support */
+     #address-cells = <1>;
+     #size-cells = <0>;
+     pcm6240: audio-codec@48 {
+       compatible = "ti,pcm6240";
+       reg = <0x48>, /* primary-device */
+             <0x4b>; /* secondary-device */
+       #sound-dai-cells = <0>;
+       reset-gpios = <&gpio1 10 GPIO_ACTIVE_HIGH>;
+       interrupt-parent = <&gpio1>;
+       interrupts = <15>;
+     };
+   };
+...
-- 
2.34.1


^ permalink raw reply related

* [PATCH v8 3/4] ASoc: PCM6240: Add compile item for PCM6240 Family driver
From: Shenghao Ding @ 2024-04-03  0:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-sound, devicetree, perex, tiwai, 13916275206, mohit.chawla,
	soyer, jkhuang3, tiwai, pdjuandi, manisha.agrawal, aviel,
	hnagalla, praneeth, Baojun.Xu, Shenghao Ding
In-Reply-To: <20240403003159.389-1-shenghao-ding@ti.com>

PCM6240 driver implements a flexible and configurable setting for register
and filter coefficients, to one, two or even multiple PCM6240 Family Audio
chips.

Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>

---
v8:
 - use some reasonable format of changelog.
 - Link to v2: https://lore.kernel.org/all/20240126035855.1785-3-shenghao-ding@ti.com/
---
 sound/soc/codecs/Kconfig  | 10 ++++++++++
 sound/soc/codecs/Makefile |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index f78ea2f86fa6..0c35cdfd4a47 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -179,6 +179,7 @@ config SND_SOC_ALL_CODECS
 	imply SND_SOC_PCM5102A
 	imply SND_SOC_PCM512x_I2C
 	imply SND_SOC_PCM512x_SPI
+	imply SND_SOC_PCM6240
 	imply SND_SOC_PEB2466
 	imply SND_SOC_RK3328
 	imply SND_SOC_RK817
@@ -1422,6 +1423,15 @@ config SND_SOC_PCM512x_SPI
 	select SND_SOC_PCM512x
 	select REGMAP_SPI
 
+config SND_SOC_PCM6240
+	tristate "Texas Instruments PCM6240 Family Audio chips based on I2C"
+	depends on I2C
+	help
+	  Enable support for Texas Instruments PCM6240 Family Audio chips.
+	  Note the PCM6240 driver implements a flexible and configurable
+	  setting for register and filter coefficients, to one, two or
+	  even multiple PCM6240 Family Audio chips.
+
 config SND_SOC_PEB2466
 	tristate "Infineon PEB2466 quad PCM codec"
 	depends on SPI
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 7c075539dc47..5553155b843b 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -204,6 +204,7 @@ snd-soc-pcm5102a-objs := pcm5102a.o
 snd-soc-pcm512x-objs := pcm512x.o
 snd-soc-pcm512x-i2c-objs := pcm512x-i2c.o
 snd-soc-pcm512x-spi-objs := pcm512x-spi.o
+snd-soc-pcm6240-objs := pcm6240.o
 snd-soc-peb2466-objs := peb2466.o
 snd-soc-rk3328-objs := rk3328_codec.o
 snd-soc-rk817-objs := rk817_codec.o
@@ -594,6 +595,7 @@ obj-$(CONFIG_SND_SOC_PCM5102A)	+= snd-soc-pcm5102a.o
 obj-$(CONFIG_SND_SOC_PCM512x)	+= snd-soc-pcm512x.o
 obj-$(CONFIG_SND_SOC_PCM512x_I2C)	+= snd-soc-pcm512x-i2c.o
 obj-$(CONFIG_SND_SOC_PCM512x_SPI)	+= snd-soc-pcm512x-spi.o
+obj-$(CONFIG_SND_SOC_PCM6240)	+= snd-soc-pcm6240.o
 obj-$(CONFIG_SND_SOC_PEB2466)	+= snd-soc-peb2466.o
 obj-$(CONFIG_SND_SOC_RK3328)	+= snd-soc-rk3328.o
 obj-$(CONFIG_SND_SOC_RK817)	+= snd-soc-rk817.o
-- 
2.34.1


^ 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