* [PATCH v2 1/3] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
From: Dmitry Baryshkov @ 2024-03-30 15:00 UTC (permalink / raw)
To: 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
Cc: dri-devel, devicetree, linux-kernel, linux-arm-msm, Vinod Koul,
Caleb Connolly
In-Reply-To: <20240330-lg-sw43408-panel-v2-0-293a58717b38@linaro.org>
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>
---
.../bindings/display/panel/lg,sw43408.yaml | 62 ++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/panel/lg,sw43408.yaml b/Documentation/devicetree/bindings/display/panel/lg,sw43408.yaml
new file mode 100644
index 000000000000..1e08648f5bc7
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/lg,sw43408.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/lg,sw43408.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LG SW43408 1080x2160 DSI panel
+
+maintainers:
+ - Caleb Connolly <caleb.connolly@linaro.org>
+
+description:
+ This panel is used on the Pixel 3, it is a 60hz OLED panel which
+ required DSC (Display Stream Compression) and has rounded corners.
+
+allOf:
+ - $ref: panel-common.yaml#
+
+properties:
+ compatible:
+ items:
+ - const: lg,sw43408
+
+ reg: true
+ port: true
+ vddi-supply: true
+ vpnl-supply: true
+ reset-gpios: true
+
+required:
+ - compatible
+ - vddi-supply
+ - vpnl-supply
+ - reset-gpios
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ dsi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ panel@0 {
+ compatible = "lg,sw43408";
+ reg = <0>;
+
+ vddi-supply = <&vreg_l14a_1p88>;
+ vpnl-supply = <&vreg_l28a_3p0>;
+
+ reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
+
+ port {
+ endpoint {
+ remote-endpoint = <&mdss_dsi0_out>;
+ };
+ };
+ };
+ };
+...
--
2.39.2
^ permalink raw reply related
* [PATCH v2 0/3] drm/panel: add support for LG SW43408 panel
From: Dmitry Baryshkov @ 2024-03-30 15:00 UTC (permalink / raw)
To: 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
Cc: dri-devel, devicetree, linux-kernel, linux-arm-msm, Vinod Koul,
Caleb Connolly
The LG SW43408 panel is used on Google Pixel3 devices. For a long time
we could not submit the driver, as the panel was not coming up from the
reset. The panel seems to be picky about some of the delays during init
and it also uses non-standard payload for MIPI_DSI_COMPRESSION_MODE.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
Changes in v2:
- Removed formatting char from schema (Krzysztof)
- Moved additionalProperties after required (Krzysztof)
- Added example to the schema (Krzysztof)
- Removed obsolete comment in the commit message (Marijn)
- Moved DSC params to the panel struct (Marijn)
- Changed dsc_en to be an array (Marijn)
- Added comment regiarding slice_width and slice_count (Marijn)
- Link to v1: https://lore.kernel.org/r/20240330-lg-sw43408-panel-v1-0-f5580fc9f2da@linaro.org
---
Dmitry Baryshkov (1):
drm/mipi-dsi: add mipi_dsi_compression_mode_raw()
Sumit Semwal (2):
dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
drm: panel: Add LG sw43408 panel driver
.../bindings/display/panel/lg,sw43408.yaml | 62 ++++
MAINTAINERS | 8 +
drivers/gpu/drm/drm_mipi_dsi.c | 34 ++-
drivers/gpu/drm/panel/Kconfig | 11 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-lg-sw43408.c | 321 +++++++++++++++++++++
include/drm/drm_mipi_dsi.h | 1 +
7 files changed, 430 insertions(+), 8 deletions(-)
---
base-commit: 13ee4a7161b6fd938aef6688ff43b163f6d83e37
change-id: 20240330-lg-sw43408-panel-b552f411c53e
Best regards,
--
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
^ permalink raw reply
* Re: [PATCH net-next v6 17/17] net: pse-pd: Add TI TPS23881 PSE controller driver
From: Andrew Lunn @ 2024-03-30 14:52 UTC (permalink / raw)
To: Kory Maincent
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jonathan Corbet, Luis Chamberlain, Russ Weight,
Greg Kroah-Hartman, Rafael J. Wysocki, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Oleksij Rempel, Mark Brown,
Frank Rowand, Heiner Kallweit, Russell King, Thomas Petazzoni,
netdev, linux-kernel, linux-doc, devicetree, Dent Project
In-Reply-To: <20240329155657.7939ac4b@kmaincent-XPS-13-7390>
On Fri, Mar 29, 2024 at 03:56:57PM +0100, Kory Maincent wrote:
> On Thu, 28 Mar 2024 17:24:17 +0100
> Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > +static int tps23881_flash_fw_part(struct i2c_client *client,
> > > + const char *fw_name,
> > > + const struct tps23881_fw_conf *fw_conf)
> >
> > Does the device actually have flash? Or is this just downloading to
> > SRAM?
>
> It is downloading to SRAM.
So maybe rename these functions.
Andrew
^ permalink raw reply
* Re: [PATCH 3/3] drm: panel: Add LG sw43408 panel driver
From: Dmitry Baryshkov @ 2024-03-30 14: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: <554zkisebym7gbbom3657ws7kqvyidggfmcvetjm6vrnwts3gl@l53hejt72b5q>
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.
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.
>
> > +
> > + 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.
> > + 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.
> > +
> > + 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,
> > +
> > + .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 },
> > + { /* 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;
> > + struct drm_dsc_config *dsc;
> > + 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 */
> > + dsc = devm_kzalloc(&dsi->dev, sizeof(*dsc), GFP_KERNEL);
>
> We've recently decided to store struct drm_dsc_config in struct sw43408_panel
> and save on an extra allocation.
>
> > + if (!dsc)
> > + return -ENOMEM;
> > +
> > + dsc->dsc_version_major = 0x1;
> > + dsc->dsc_version_minor = 0x1;
> > +
> > + dsc->slice_height = 16;
> > + dsc->slice_width = 540;
> > + dsc->slice_count = 2;
>
> Maybe incorporate with a comment that slice_count * slice_width == the width of
> the mode?
ack
>
> - Marijn
>
> > + dsc->bits_per_component = 8;
> > + dsc->bits_per_pixel = 8 << 4;
> > + dsc->block_pred_enable = true;
> > +
> > + dsi->dsc = 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 v7 2/2] dmaengine: Loongson1: Add Loongson-1 APB DMA driver
From: Huacai Chen @ 2024-03-30 13:59 UTC (permalink / raw)
To: keguang.zhang
Cc: Vinod Koul, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-mips, dmaengine, devicetree, linux-kernel
In-Reply-To: <20240329-loongson1-dma-v7-2-37db58608de5@gmail.com>
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)?
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
>
>
^ permalink raw reply
* Re: [RFC PATCH 0/2] Add gpio-usb-c-connector compatible
From: Dmitry Baryshkov @ 2024-03-30 13:50 UTC (permalink / raw)
To: Krishna Kurapati PSSNV
Cc: Rob Herring, Krzysztof Kozlowski, Greg Kroah-Hartman,
Conor Dooley, Miquel Raynal, Guenter Roeck, Bjorn Helgaas,
Kyle Tso, Fabrice Gasnier, Heikki Krogerus, u.kleine-koenig,
AngeloGioacchino Del Regno, devicetree, linux-usb, linux-kernel,
quic_ppratap, quic_jackp
In-Reply-To: <44bc6ea4-eba9-4b80-bb07-3b744eb7cce6@quicinc.com>
On Sat, 30 Mar 2024 at 15:46, Krishna Kurapati PSSNV
<quic_kriskura@quicinc.com> wrote:
>
>
>
> On 3/30/2024 7:09 PM, Dmitry Baryshkov wrote:
> > On Sat, 30 Mar 2024 at 11:13, Krishna Kurapati PSSNV
> > <quic_kriskura@quicinc.com> wrote:
> >> On 3/29/2024 6:23 PM, Dmitry Baryshkov wrote:
> >>> On Fri, 29 Mar 2024 at 09:20, Krishna Kurapati
> >>> <quic_kriskura@quicinc.com> wrote:
> >>>>
> >>>> QDU1000 IDP [1] has a Type-c connector and supports USB 3.0.
> >>>> However it relies on usb-conn-gpio driver to read the vbus and id
> >>>> gpio's and provide role switch. However the driver currently has
> >>>> only gpio-b-connector compatible present in ID table. Adding that
> >>>> in DT would mean that the device supports Type-B connector and not
> >>>> Type-c connector. Thanks to Dmitry Baryshkov for pointing it out [2].
> >>>
> >>> USB-B connector is pretty simple, it really has just an ID pin and
> >>> VBUS input, which translates to two GPIOs being routed from the
> >>> _connector_ itself.
> >>>
> >>> USB-C is much more complicated, it has two CC pins and a VBus power
> >>> pin. It is not enough just to measure CC pin levels. Moreover,
> >>> properly handling USB 3.0 inside a USB-C connector requires a separate
> >>> 'orientation' signal to tell the host which two lanes must be used for
> >>> the USB SS signals. Thus it is no longer possible to route just two
> >>> pins from the connector to the SoC.
> >>>
> >>> Having all that in mind, I suspect that you are not describing your
> >>> hardware properly. I suppose that you have a Type-C port controller /
> >>> redriver / switch, which handles CC lines communication and then
> >>> provides ID / VBUS signals to the host. In such a case, please
> >>> describe this TCPC in the DT file and use its compatible string
> >>> instead of "gpio-c-connector".
> >>>
> >>
> >> Hi Dmitry,
> >>
> >> My bad. I must have provided more details of the HW.
> >>
> >> I presume you are referring to addition of a connector node, type-c
> >> switch, pmic-glink and other remote endpoints like in other SoC's like
> >> SM8450/ SM8550/ SM8650.
> >>
> >> This HW is slightly different. It has a Uni Phy for Super speed and
> >> hence no DP.
> >
> > This is fine and it's irrelevant for the USB-C.
> >
> >> For orientation switching, on mobile SoC's, there is a provision for
> >> orientation gpio given in pmic-glink node and is handled in ucsi_glink
> >> driver. But on this version of HW, there is a USB-C Switch with its own
> >> firmware taking care of orientation switching. It takes 8 SS Lines and 2
> >> CC lines coming from connector as input and gives out 4 SS Lines (SS
> >> TX1/TX2 RX1/RX2) as output which go to the SoC. So orientation switch is
> >> done by the USB-C-switch in between and it automatically routes
> >> appropriate active SS Lane from connector to the SoC.
> >
> > This is also fine. As I wrote, you _have_ the Type-C port controller.
> > So your DT file should be describing your hardware.
> >
> >> As usual like in other targets, the DP and DM lines from type-c
> >> connector go to the SoC directly.
> >>
> >> To handle role switch, the VBUS and ID Pin connections are given to
> >> SoC as well. There is a vbus controller regulator present to provide
> >> vbus to connected peripherals in host mode.
> >>
> >> There is no PPM entity (ADSP in mobile SoC's) and no UCSI involved
> >> here. Hence we rely on usb-conn-gpio to read the vbus/id and switch
> >> roles accordingly.
> >
> > This is also fine.
> >
> > You confirmed my suspicions. You have an external Type-C switch which
> > handles orientation (and most likely PD or non-PD power negotiation)
> > for you. It has GPIO outputs, etc.
> >
> > But it is not a part of the connector. Instead of adding the
> > "gpio-usb-c-connector", add proper compatible string (see, how this is
> > handled e.g. by the spidev - it is a generic driver, but it requires
> > hardware-specific compatibles).
> > Your hardware description should look like:
> >
> > typec {
> > compatible = "your,switch";
> > id-gpios = <&gpio 1>;
> > vbus-gpios = <&gpio 2>;
> > vbus-supplies = <®-vbus>;
> >
> > ports {
> > #address-cells = <1>;
> > #size-cells = <1>;
> > port@0 {
> > endpoint {
> > remote-endpoint = <&usb_dwc3_hs_out>;
> > };
> > };
> > port@1 {
> > endpoint {
> > remote-endpoint = <&usb_uni_phy_out>;
> > };
> > };
> > /* No SBU port */
> > };
> > };
> > > Note, I haven't said anything regarding the driver. You can continue
> > using the usb-conn-gpio driver. Just add a compatible string for you
> > switch.
> >
>
>
> Got it. So the "usb_conn_gpio: usb-conn-gpio" in [1] to be replaced
> with something like a "typec- " naming convention and add a new
> compatible to gpio-conn (something specific to qcom-qdu) and use it in
> the new DT node.
It should be the actual name of the switch chip.
>
> Thanks for the suggestion. Is it fine if it put the whole of the above
> text in v2 and push it for getting a new compatible added to connector
> binding and usb-conn driver and then send v3 of DT changes or mix this
> series with the DT series ?
I think USB subsystem maintainers prefer separate series.
>
> [1]:
> https://lore.kernel.org/all/20240319091020.15137-3-quic_kbajaj@quicinc.com/
>
> Thanks,
> Krishna,
>
> >>
> >> Hope this answers the query as to why we wanted to use usb-conn-gpio
> >> and why we were trying to add a new compatible.
> >>
> >> Regards,
> >> Krishna,
> >>
> >>>>
> >>>> This series intends to add that compatible in driver and bindings
> >>>> so that it can be used in QDU1000 IDP DT.
> >>>>
> >>>> [1]: https://lore.kernel.org/all/20240319091020.15137-3-quic_kbajaj@quicinc.com/
> >>>> [2]: https://lore.kernel.org/all/CAA8EJprXPvji8TgZu1idH7y4GtHtD4VmQABFBcRt-9BQaCberg@mail.gmail.com/
> >>>>
> >>>> Krishna Kurapati (2):
> >>>> dt-bindings: connector: Add gpio-usb-c-connector compatible
> >>>> usb: common: usb-conn-gpio: Update ID table to add usb-c connector
> >>>>
> >>>> Documentation/devicetree/bindings/connector/usb-connector.yaml | 3 +++
> >>>> drivers/usb/common/usb-conn-gpio.c | 1 +
> >>>> 2 files changed, 4 insertions(+)
> >>>>
> >>>> --
> >>>> 2.34.1
> >>>>
> >>>
> >>>
> >>> --
> >>> With best wishes
> >>> Dmitry
> >
> >
> >
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [RFC PATCH 0/2] Add gpio-usb-c-connector compatible
From: Krishna Kurapati PSSNV @ 2024-03-30 13:45 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Herring, Krzysztof Kozlowski, Greg Kroah-Hartman,
Conor Dooley, Miquel Raynal, Guenter Roeck, Bjorn Helgaas,
Kyle Tso, Fabrice Gasnier, Heikki Krogerus, u.kleine-koenig,
AngeloGioacchino Del Regno, devicetree, linux-usb, linux-kernel,
quic_ppratap, quic_jackp
In-Reply-To: <CAA8EJppa4hVBSenLgxc5MYxTfzPPf4exHvh8RWTP=p8mgB_RCw@mail.gmail.com>
On 3/30/2024 7:09 PM, Dmitry Baryshkov wrote:
> On Sat, 30 Mar 2024 at 11:13, Krishna Kurapati PSSNV
> <quic_kriskura@quicinc.com> wrote:
>> On 3/29/2024 6:23 PM, Dmitry Baryshkov wrote:
>>> On Fri, 29 Mar 2024 at 09:20, Krishna Kurapati
>>> <quic_kriskura@quicinc.com> wrote:
>>>>
>>>> QDU1000 IDP [1] has a Type-c connector and supports USB 3.0.
>>>> However it relies on usb-conn-gpio driver to read the vbus and id
>>>> gpio's and provide role switch. However the driver currently has
>>>> only gpio-b-connector compatible present in ID table. Adding that
>>>> in DT would mean that the device supports Type-B connector and not
>>>> Type-c connector. Thanks to Dmitry Baryshkov for pointing it out [2].
>>>
>>> USB-B connector is pretty simple, it really has just an ID pin and
>>> VBUS input, which translates to two GPIOs being routed from the
>>> _connector_ itself.
>>>
>>> USB-C is much more complicated, it has two CC pins and a VBus power
>>> pin. It is not enough just to measure CC pin levels. Moreover,
>>> properly handling USB 3.0 inside a USB-C connector requires a separate
>>> 'orientation' signal to tell the host which two lanes must be used for
>>> the USB SS signals. Thus it is no longer possible to route just two
>>> pins from the connector to the SoC.
>>>
>>> Having all that in mind, I suspect that you are not describing your
>>> hardware properly. I suppose that you have a Type-C port controller /
>>> redriver / switch, which handles CC lines communication and then
>>> provides ID / VBUS signals to the host. In such a case, please
>>> describe this TCPC in the DT file and use its compatible string
>>> instead of "gpio-c-connector".
>>>
>>
>> Hi Dmitry,
>>
>> My bad. I must have provided more details of the HW.
>>
>> I presume you are referring to addition of a connector node, type-c
>> switch, pmic-glink and other remote endpoints like in other SoC's like
>> SM8450/ SM8550/ SM8650.
>>
>> This HW is slightly different. It has a Uni Phy for Super speed and
>> hence no DP.
>
> This is fine and it's irrelevant for the USB-C.
>
>> For orientation switching, on mobile SoC's, there is a provision for
>> orientation gpio given in pmic-glink node and is handled in ucsi_glink
>> driver. But on this version of HW, there is a USB-C Switch with its own
>> firmware taking care of orientation switching. It takes 8 SS Lines and 2
>> CC lines coming from connector as input and gives out 4 SS Lines (SS
>> TX1/TX2 RX1/RX2) as output which go to the SoC. So orientation switch is
>> done by the USB-C-switch in between and it automatically routes
>> appropriate active SS Lane from connector to the SoC.
>
> This is also fine. As I wrote, you _have_ the Type-C port controller.
> So your DT file should be describing your hardware.
>
>> As usual like in other targets, the DP and DM lines from type-c
>> connector go to the SoC directly.
>>
>> To handle role switch, the VBUS and ID Pin connections are given to
>> SoC as well. There is a vbus controller regulator present to provide
>> vbus to connected peripherals in host mode.
>>
>> There is no PPM entity (ADSP in mobile SoC's) and no UCSI involved
>> here. Hence we rely on usb-conn-gpio to read the vbus/id and switch
>> roles accordingly.
>
> This is also fine.
>
> You confirmed my suspicions. You have an external Type-C switch which
> handles orientation (and most likely PD or non-PD power negotiation)
> for you. It has GPIO outputs, etc.
>
> But it is not a part of the connector. Instead of adding the
> "gpio-usb-c-connector", add proper compatible string (see, how this is
> handled e.g. by the spidev - it is a generic driver, but it requires
> hardware-specific compatibles).
> Your hardware description should look like:
>
> typec {
> compatible = "your,switch";
> id-gpios = <&gpio 1>;
> vbus-gpios = <&gpio 2>;
> vbus-supplies = <®-vbus>;
>
> ports {
> #address-cells = <1>;
> #size-cells = <1>;
> port@0 {
> endpoint {
> remote-endpoint = <&usb_dwc3_hs_out>;
> };
> };
> port@1 {
> endpoint {
> remote-endpoint = <&usb_uni_phy_out>;
> };
> };
> /* No SBU port */
> };
> };
> > Note, I haven't said anything regarding the driver. You can continue
> using the usb-conn-gpio driver. Just add a compatible string for you
> switch.
>
Got it. So the "usb_conn_gpio: usb-conn-gpio" in [1] to be replaced
with something like a "typec- " naming convention and add a new
compatible to gpio-conn (something specific to qcom-qdu) and use it in
the new DT node.
Thanks for the suggestion. Is it fine if it put the whole of the above
text in v2 and push it for getting a new compatible added to connector
binding and usb-conn driver and then send v3 of DT changes or mix this
series with the DT series ?
[1]:
https://lore.kernel.org/all/20240319091020.15137-3-quic_kbajaj@quicinc.com/
Thanks,
Krishna,
>>
>> Hope this answers the query as to why we wanted to use usb-conn-gpio
>> and why we were trying to add a new compatible.
>>
>> Regards,
>> Krishna,
>>
>>>>
>>>> This series intends to add that compatible in driver and bindings
>>>> so that it can be used in QDU1000 IDP DT.
>>>>
>>>> [1]: https://lore.kernel.org/all/20240319091020.15137-3-quic_kbajaj@quicinc.com/
>>>> [2]: https://lore.kernel.org/all/CAA8EJprXPvji8TgZu1idH7y4GtHtD4VmQABFBcRt-9BQaCberg@mail.gmail.com/
>>>>
>>>> Krishna Kurapati (2):
>>>> dt-bindings: connector: Add gpio-usb-c-connector compatible
>>>> usb: common: usb-conn-gpio: Update ID table to add usb-c connector
>>>>
>>>> Documentation/devicetree/bindings/connector/usb-connector.yaml | 3 +++
>>>> drivers/usb/common/usb-conn-gpio.c | 1 +
>>>> 2 files changed, 4 insertions(+)
>>>>
>>>> --
>>>> 2.34.1
>>>>
>>>
>>>
>>> --
>>> With best wishes
>>> Dmitry
>
>
>
^ permalink raw reply
* Re: [RFC PATCH 0/2] Add gpio-usb-c-connector compatible
From: Dmitry Baryshkov @ 2024-03-30 13:39 UTC (permalink / raw)
To: Krishna Kurapati PSSNV
Cc: Rob Herring, Krzysztof Kozlowski, Greg Kroah-Hartman,
Conor Dooley, Miquel Raynal, Guenter Roeck, Bjorn Helgaas,
Kyle Tso, Fabrice Gasnier, Heikki Krogerus, u.kleine-koenig,
AngeloGioacchino Del Regno, devicetree, linux-usb, linux-kernel,
quic_ppratap, quic_jackp
In-Reply-To: <6f2df222-36d4-468e-99a7-9c48fae85aa9@quicinc.com>
On Sat, 30 Mar 2024 at 11:13, Krishna Kurapati PSSNV
<quic_kriskura@quicinc.com> wrote:
> On 3/29/2024 6:23 PM, Dmitry Baryshkov wrote:
> > On Fri, 29 Mar 2024 at 09:20, Krishna Kurapati
> > <quic_kriskura@quicinc.com> wrote:
> >>
> >> QDU1000 IDP [1] has a Type-c connector and supports USB 3.0.
> >> However it relies on usb-conn-gpio driver to read the vbus and id
> >> gpio's and provide role switch. However the driver currently has
> >> only gpio-b-connector compatible present in ID table. Adding that
> >> in DT would mean that the device supports Type-B connector and not
> >> Type-c connector. Thanks to Dmitry Baryshkov for pointing it out [2].
> >
> > USB-B connector is pretty simple, it really has just an ID pin and
> > VBUS input, which translates to two GPIOs being routed from the
> > _connector_ itself.
> >
> > USB-C is much more complicated, it has two CC pins and a VBus power
> > pin. It is not enough just to measure CC pin levels. Moreover,
> > properly handling USB 3.0 inside a USB-C connector requires a separate
> > 'orientation' signal to tell the host which two lanes must be used for
> > the USB SS signals. Thus it is no longer possible to route just two
> > pins from the connector to the SoC.
> >
> > Having all that in mind, I suspect that you are not describing your
> > hardware properly. I suppose that you have a Type-C port controller /
> > redriver / switch, which handles CC lines communication and then
> > provides ID / VBUS signals to the host. In such a case, please
> > describe this TCPC in the DT file and use its compatible string
> > instead of "gpio-c-connector".
> >
>
> Hi Dmitry,
>
> My bad. I must have provided more details of the HW.
>
> I presume you are referring to addition of a connector node, type-c
> switch, pmic-glink and other remote endpoints like in other SoC's like
> SM8450/ SM8550/ SM8650.
>
> This HW is slightly different. It has a Uni Phy for Super speed and
> hence no DP.
This is fine and it's irrelevant for the USB-C.
> For orientation switching, on mobile SoC's, there is a provision for
> orientation gpio given in pmic-glink node and is handled in ucsi_glink
> driver. But on this version of HW, there is a USB-C Switch with its own
> firmware taking care of orientation switching. It takes 8 SS Lines and 2
> CC lines coming from connector as input and gives out 4 SS Lines (SS
> TX1/TX2 RX1/RX2) as output which go to the SoC. So orientation switch is
> done by the USB-C-switch in between and it automatically routes
> appropriate active SS Lane from connector to the SoC.
This is also fine. As I wrote, you _have_ the Type-C port controller.
So your DT file should be describing your hardware.
> As usual like in other targets, the DP and DM lines from type-c
> connector go to the SoC directly.
>
> To handle role switch, the VBUS and ID Pin connections are given to
> SoC as well. There is a vbus controller regulator present to provide
> vbus to connected peripherals in host mode.
>
> There is no PPM entity (ADSP in mobile SoC's) and no UCSI involved
> here. Hence we rely on usb-conn-gpio to read the vbus/id and switch
> roles accordingly.
This is also fine.
You confirmed my suspicions. You have an external Type-C switch which
handles orientation (and most likely PD or non-PD power negotiation)
for you. It has GPIO outputs, etc.
But it is not a part of the connector. Instead of adding the
"gpio-usb-c-connector", add proper compatible string (see, how this is
handled e.g. by the spidev - it is a generic driver, but it requires
hardware-specific compatibles).
Your hardware description should look like:
typec {
compatible = "your,switch";
id-gpios = <&gpio 1>;
vbus-gpios = <&gpio 2>;
vbus-supplies = <®-vbus>;
ports {
#address-cells = <1>;
#size-cells = <1>;
port@0 {
endpoint {
remote-endpoint = <&usb_dwc3_hs_out>;
};
};
port@1 {
endpoint {
remote-endpoint = <&usb_uni_phy_out>;
};
};
/* No SBU port */
};
};
Note, I haven't said anything regarding the driver. You can continue
using the usb-conn-gpio driver. Just add a compatible string for you
switch.
>
> Hope this answers the query as to why we wanted to use usb-conn-gpio
> and why we were trying to add a new compatible.
>
> Regards,
> Krishna,
>
> >>
> >> This series intends to add that compatible in driver and bindings
> >> so that it can be used in QDU1000 IDP DT.
> >>
> >> [1]: https://lore.kernel.org/all/20240319091020.15137-3-quic_kbajaj@quicinc.com/
> >> [2]: https://lore.kernel.org/all/CAA8EJprXPvji8TgZu1idH7y4GtHtD4VmQABFBcRt-9BQaCberg@mail.gmail.com/
> >>
> >> Krishna Kurapati (2):
> >> dt-bindings: connector: Add gpio-usb-c-connector compatible
> >> usb: common: usb-conn-gpio: Update ID table to add usb-c connector
> >>
> >> Documentation/devicetree/bindings/connector/usb-connector.yaml | 3 +++
> >> drivers/usb/common/usb-conn-gpio.c | 1 +
> >> 2 files changed, 4 insertions(+)
> >>
> >> --
> >> 2.34.1
> >>
> >
> >
> > --
> > With best wishes
> > Dmitry
--
With best wishes
Dmitry
^ permalink raw reply
* [PATCH v2 3/3] arm64: dts: freescale: Add device tree for Emcraft Systems NavQ+ Kit
From: Gilles Talis @ 2024-03-30 13:34 UTC (permalink / raw)
To: devicetree, imx, linux-arm-kernel
Cc: conor+dt, krzysztof.kozlowski+dt, robh, shawnguo, festevam, alex,
andrew, Gilles Talis
In-Reply-To: <20240330133410.41408-1-gilles.talis@gmail.com>
The Emcraft Systems NavQ+ kit is a mobile robotics platform
based on NXP i.MX8 MPlus SoC.
The following interfaces and devices are enabled:
- eMMC
- Gigabit Ethernet
- RTC
- SD-Card
- UART console
Signed-off-by: Gilles Talis <gilles.talis@gmail.com>
---
arch/arm64/boot/dts/freescale/Makefile | 1 +
.../arm64/boot/dts/freescale/imx8mp-navqp.dts | 424 ++++++++++++++++++
2 files changed, 425 insertions(+)
create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-navqp.dts
diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 045250d0a040..bf99864c0bc4 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -166,6 +166,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-pdk3.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mp-msc-sm2s-ep1.dtb
+dtb-$(CONFIG_ARCH_MXC) += imx8mp-navqp.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mp-skov-revb-hdmi.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mp-skov-revb-lt6.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-navqp.dts b/arch/arm64/boot/dts/freescale/imx8mp-navqp.dts
new file mode 100644
index 000000000000..5fd1614982cd
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mp-navqp.dts
@@ -0,0 +1,424 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2021 Emcraft Systems
+ * Copyright 2024 Gilles Talis <gilles.talis@gmail.com>
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+#include "imx8mp.dtsi"
+
+/ {
+ model = "Emcraft Systems i.MX8MPlus NavQ+ Kit";
+ compatible = "emcraft,imx8mp-navqp", "fsl,imx8mp";
+
+ chosen {
+ stdout-path = &uart2;
+ };
+
+ leds {
+ compatible = "gpio-leds";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_gpio_led>;
+
+ led-0 {
+ color = <LED_COLOR_ID_GREEN>;
+ function = LED_FUNCTION_STATUS;
+ gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>;
+ default-state = "on";
+ };
+ };
+
+ reg_usdhc2_vmmc: regulator-usdhc2 {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>;
+ regulator-name = "VSD_3V3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ startup-delay-us = <100>;
+ off-on-delay-us = <12000>;
+ };
+};
+
+&A53_0 {
+ cpu-supply = <&buck2>;
+};
+
+&A53_1 {
+ cpu-supply = <&buck2>;
+};
+
+&A53_2 {
+ cpu-supply = <&buck2>;
+};
+
+&A53_3 {
+ cpu-supply = <&buck2>;
+};
+
+&eqos {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_eqos>;
+ phy-mode = "rgmii-id";
+ phy-handle = <ðphy0>;
+ status = "okay";
+
+ mdio {
+ compatible = "snps,dwmac-mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethphy0: ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <0>;
+ reset-gpios = <&gpio4 22 GPIO_ACTIVE_LOW>;
+ reset-assert-us = <1000>;
+ reset-deassert-us = <10000>;
+ qca,disable-smarteee;
+ qca,disable-hibernation-mode;
+ };
+ };
+};
+
+&i2c1 {
+ clock-frequency = <400000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i2c1>;
+ status = "okay";
+
+ pmic@25 {
+ compatible = "nxp,pca9450c";
+ reg = <0x25>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pmic>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+ regulators {
+ BUCK1 {
+ regulator-name = "BUCK1";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <2187500>;
+ regulator-boot-on;
+ regulator-always-on;
+ regulator-ramp-delay = <3125>;
+ };
+
+ buck2: BUCK2 {
+ regulator-name = "BUCK2";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <2187500>;
+ regulator-boot-on;
+ regulator-always-on;
+ regulator-ramp-delay = <3125>;
+ nxp,dvs-run-voltage = <950000>;
+ nxp,dvs-standby-voltage = <850000>;
+ };
+
+ BUCK4 {
+ regulator-name = "BUCK4";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <3400000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ BUCK5 {
+ regulator-name = "BUCK5";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <3400000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ BUCK6 {
+ regulator-name = "BUCK6";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <3400000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ LDO1 {
+ regulator-name = "LDO1";
+ regulator-min-microvolt = <1600000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ LDO2 {
+ regulator-name = "LDO2";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <1150000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ LDO3 {
+ regulator-name = "LDO3";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ LDO4 {
+ regulator-name = "LDO4";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ LDO5 {
+ regulator-name = "LDO5";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+ };
+ };
+};
+
+&i2c2 {
+ clock-frequency = <400000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i2c2>;
+ status = "okay";
+};
+
+&i2c3 {
+ clock-frequency = <400000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i2c3>;
+ status = "okay";
+};
+
+&i2c4 {
+ clock-frequency = <400000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i2c4>;
+ status = "okay";
+
+ rtc@53 {
+ compatible = "nxp,pcf2131";
+ reg = <0x53>;
+ };
+};
+
+&uart2 {
+ /* console */
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart2>;
+ status = "okay";
+};
+
+/* SD Card */
+&usdhc2 {
+ pinctrl-names = "default", "state_100mhz", "state_200mhz";
+ pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
+ pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>;
+ pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_gpio>;
+ cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
+ vmmc-supply = <®_usdhc2_vmmc>;
+ bus-width = <4>;
+ status = "okay";
+};
+
+/* eMMC */
+&usdhc3 {
+ assigned-clocks = <&clk IMX8MP_CLK_USDHC3>;
+ assigned-clock-rates = <400000000>;
+ pinctrl-names = "default", "state_100mhz", "state_200mhz";
+ pinctrl-0 = <&pinctrl_usdhc3>;
+ pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
+ pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
+ bus-width = <8>;
+ non-removable;
+ status = "okay";
+};
+
+&wdog1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdog>;
+ fsl,ext-reset-output;
+ status = "okay";
+};
+
+&iomuxc {
+ pinctrl_eqos: eqosgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_ENET_MDC__ENET_QOS_MDC 0x3
+ MX8MP_IOMUXC_ENET_MDIO__ENET_QOS_MDIO 0x3
+ MX8MP_IOMUXC_ENET_RD0__ENET_QOS_RGMII_RD0 0x91
+ MX8MP_IOMUXC_ENET_RD1__ENET_QOS_RGMII_RD1 0x91
+ MX8MP_IOMUXC_ENET_RD2__ENET_QOS_RGMII_RD2 0x91
+ MX8MP_IOMUXC_ENET_RD3__ENET_QOS_RGMII_RD3 0x91
+ MX8MP_IOMUXC_ENET_RXC__CCM_ENET_QOS_CLOCK_GENERATE_RX_CLK 0x91
+ MX8MP_IOMUXC_ENET_RX_CTL__ENET_QOS_RGMII_RX_CTL 0x91
+ MX8MP_IOMUXC_ENET_TD0__ENET_QOS_RGMII_TD0 0x1f
+ MX8MP_IOMUXC_ENET_TD1__ENET_QOS_RGMII_TD1 0x1f
+ MX8MP_IOMUXC_ENET_TD2__ENET_QOS_RGMII_TD2 0x1f
+ MX8MP_IOMUXC_ENET_TD3__ENET_QOS_RGMII_TD3 0x1f
+ MX8MP_IOMUXC_ENET_TX_CTL__ENET_QOS_RGMII_TX_CTL 0x1f
+ MX8MP_IOMUXC_ENET_TXC__CCM_ENET_QOS_CLOCK_GENERATE_TX_CLK 0x1f
+ MX8MP_IOMUXC_SAI2_RXC__GPIO4_IO22 0x110
+ >;
+ };
+
+ pinctrl_gpio_led: gpioledgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_NAND_READY_B__GPIO3_IO16 0x19
+ >;
+ };
+
+ pinctrl_i2c1: i2c1grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL 0x400001c3
+ MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA 0x400001c3
+ >;
+ };
+
+ pinctrl_i2c2: i2c2grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL 0x400001c3
+ MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA 0x400001c3
+ >;
+ };
+
+ pinctrl_i2c3: i2c3grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_I2C3_SCL__I2C3_SCL 0x400001c3
+ MX8MP_IOMUXC_I2C3_SDA__I2C3_SDA 0x400001c3
+ >;
+ };
+
+ pinctrl_i2c4: i2c4grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_I2C4_SCL__I2C4_SCL 0x400001c3
+ MX8MP_IOMUXC_I2C4_SDA__I2C4_SDA 0x400001c3
+ >;
+ };
+
+ pinctrl_pmic: pmicgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_GPIO1_IO03__GPIO1_IO03 0x41
+ >;
+ };
+
+ pinctrl_reg_usdhc2_vmmc: regusdhc2vmmcgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19 0x41
+ >;
+ };
+
+ pinctrl_uart2: uart2grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX 0x49
+ MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX 0x49
+ >;
+ };
+
+ pinctrl_usdhc2: usdhc2grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK 0x190
+ MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD 0x1d0
+ MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0 0x1d0
+ MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1 0x1d0
+ MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2 0x1d0
+ MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3 0x1d0
+ MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT 0xc1
+ >;
+ };
+
+ pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK 0x194
+ MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD 0x1d4
+ MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0 0x1d4
+ MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1 0x1d4
+ MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2 0x1d4
+ MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3 0x1d4
+ MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT 0xc1
+ >;
+ };
+
+ pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK 0x196
+ MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD 0x1d6
+ MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0 0x1d6
+ MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1 0x1d6
+ MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2 0x1d6
+ MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3 0x1d6
+ MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT 0xc1
+ >;
+ };
+
+ pinctrl_usdhc2_gpio: usdhc2gpiogrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SD2_CD_B__GPIO2_IO12 0x1c4
+ >;
+ };
+
+ pinctrl_usdhc3: usdhc3grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK 0x190
+ MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD 0x1d0
+ MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0 0x1d0
+ MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1 0x1d0
+ MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2 0x1d0
+ MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3 0x1d0
+ MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4 0x1d0
+ MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5 0x1d0
+ MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6 0x1d0
+ MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7 0x1d0
+ MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE 0x190
+ >;
+ };
+
+ pinctrl_usdhc3_100mhz: usdhc3-100mhzgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK 0x194
+ MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD 0x1d4
+ MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0 0x1d4
+ MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1 0x1d4
+ MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2 0x1d4
+ MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3 0x1d4
+ MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4 0x1d4
+ MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5 0x1d4
+ MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6 0x1d4
+ MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7 0x1d4
+ MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE 0x194
+ >;
+ };
+
+ pinctrl_usdhc3_200mhz: usdhc3-200mhzgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_NAND_WE_B__USDHC3_CLK 0x196
+ MX8MP_IOMUXC_NAND_WP_B__USDHC3_CMD 0x1d6
+ MX8MP_IOMUXC_NAND_DATA04__USDHC3_DATA0 0x1d6
+ MX8MP_IOMUXC_NAND_DATA05__USDHC3_DATA1 0x1d6
+ MX8MP_IOMUXC_NAND_DATA06__USDHC3_DATA2 0x1d6
+ MX8MP_IOMUXC_NAND_DATA07__USDHC3_DATA3 0x1d6
+ MX8MP_IOMUXC_NAND_RE_B__USDHC3_DATA4 0x1d6
+ MX8MP_IOMUXC_NAND_CE2_B__USDHC3_DATA5 0x1d6
+ MX8MP_IOMUXC_NAND_CE3_B__USDHC3_DATA6 0x1d6
+ MX8MP_IOMUXC_NAND_CLE__USDHC3_DATA7 0x1d6
+ MX8MP_IOMUXC_NAND_CE1_B__USDHC3_STROBE 0x196
+ >;
+ };
+
+ pinctrl_wdog: wdoggrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_GPIO1_IO02__WDOG1_WDOG_B 0xc6
+ >;
+ };
+};
--
2.39.2
^ permalink raw reply related
* [PATCH v2 2/3] dt-bindings: arm: Add Emcraft Systems i.MX8M Plus NavQ+ Kit
From: Gilles Talis @ 2024-03-30 13:34 UTC (permalink / raw)
To: devicetree, imx, linux-arm-kernel
Cc: conor+dt, krzysztof.kozlowski+dt, robh, shawnguo, festevam, alex,
andrew, Gilles Talis, Krzysztof Kozlowski
In-Reply-To: <20240330133410.41408-1-gilles.talis@gmail.com>
Add DT compatible string for Emcraft NavQ+ kit based on
the i.MX8M Plus SoC from NXP
Signed-off-by: Gilles Talis <gilles.talis@gmail.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Documentation/devicetree/bindings/arm/fsl.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 0027201e19f8..cec1b31d0792 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -1050,6 +1050,7 @@ properties:
- enum:
- beacon,imx8mp-beacon-kit # i.MX8MP Beacon Development Kit
- dmo,imx8mp-data-modul-edm-sbc # i.MX8MP eDM SBC
+ - emcraft,imx8mp-navqp # i.MX8MP Emcraft Systems NavQ+ Kit
- fsl,imx8mp-evk # i.MX8MP EVK Board
- gateworks,imx8mp-gw71xx-2x # i.MX8MP Gateworks Board
- gateworks,imx8mp-gw72xx-2x # i.MX8MP Gateworks Board
--
2.39.2
^ permalink raw reply related
* [PATCH v2 1/3] dt-bindings: vendor-prefixes: Add Emcraft Systems
From: Gilles Talis @ 2024-03-30 13:34 UTC (permalink / raw)
To: devicetree, imx, linux-arm-kernel
Cc: conor+dt, krzysztof.kozlowski+dt, robh, shawnguo, festevam, alex,
andrew, Gilles Talis, Krzysztof Kozlowski
In-Reply-To: <20240330133410.41408-1-gilles.talis@gmail.com>
Add an entry for Emcraft Systems (https://www.emcraft.com/)
Signed-off-by: Gilles Talis <gilles.talis@gmail.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index b97d298b3eb6..8b978c6f1dfd 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -438,6 +438,8 @@ patternProperties:
description: Dongguan EmbedFire Electronic Technology Co., Ltd.
"^embest,.*":
description: Shenzhen Embest Technology Co., Ltd.
+ "^emcraft,.*":
+ description: Emcraft Systems
"^emlid,.*":
description: Emlid, Ltd.
"^emmicro,.*":
--
2.39.2
^ permalink raw reply related
* [PATCH v2 0/3] Add support for Emcraft Systems NavQ+ kit
From: Gilles Talis @ 2024-03-30 13:34 UTC (permalink / raw)
To: devicetree, imx, linux-arm-kernel
Cc: conor+dt, krzysztof.kozlowski+dt, robh, shawnguo, festevam, alex,
andrew, Gilles Talis
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
.../devicetree/bindings/arm/fsl.yaml | 1 +
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
arch/arm64/boot/dts/freescale/Makefile | 1 +
.../arm64/boot/dts/freescale/imx8mp-navqp.dts | 424 ++++++++++++++++++
4 files changed, 428 insertions(+)
create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-navqp.dts
base-commit: 4cece764965020c22cff7665b18a012006359095
--
2.39.2
^ permalink raw reply
* [net-next,v2] dt-bindings: net: renesas,ethertsn: Create child-node for MDIO bus
From: Niklas Söderlund @ 2024-03-30 13:12 UTC (permalink / raw)
To: Sergey Shtylyov, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, netdev, devicetree
Cc: linux-renesas-soc, Niklas Söderlund, Rob Herring
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.
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Rob Herring <robh@kernel.org>
---
* Changes since v1
- Expand on history in commit message.
Hello,
The Ethernet TSN driver is still in review and have not been merged and
no usage of the bindings are merged either. So while this breaks the
binding it effects no one. So we can correct this mistake without
breaking any use-cases before we need to support any backward
compatibility.
---
.../bindings/net/renesas,ethertsn.yaml | 33 ++++++++-----------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/renesas,ethertsn.yaml b/Documentation/devicetree/bindings/net/renesas,ethertsn.yaml
index ea35d19be829..b4680a1d0a06 100644
--- a/Documentation/devicetree/bindings/net/renesas,ethertsn.yaml
+++ b/Documentation/devicetree/bindings/net/renesas,ethertsn.yaml
@@ -71,16 +71,8 @@ properties:
enum: [0, 2000]
default: 0
- '#address-cells':
- const: 1
-
- '#size-cells':
- const: 0
-
-patternProperties:
- "^ethernet-phy@[0-9a-f]$":
- type: object
- $ref: ethernet-phy.yaml#
+ mdio:
+ $ref: /schemas/net/mdio.yaml#
unevaluatedProperties: false
required:
@@ -94,8 +86,7 @@ required:
- resets
- phy-mode
- phy-handle
- - '#address-cells'
- - '#size-cells'
+ - mdio
additionalProperties: false
@@ -122,14 +113,18 @@ examples:
tx-internal-delay-ps = <2000>;
phy-handle = <&phy3>;
- #address-cells = <1>;
- #size-cells = <0>;
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
- phy3: ethernet-phy@3 {
- compatible = "ethernet-phy-ieee802.3-c45";
- reg = <0>;
- interrupt-parent = <&gpio4>;
- interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
reset-gpios = <&gpio1 23 GPIO_ACTIVE_LOW>;
+ reset-post-delay-us = <4000>;
+
+ phy3: ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c45";
+ reg = <0>;
+ interrupt-parent = <&gpio4>;
+ interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+ };
};
};
--
2.44.0
^ permalink raw reply related
* Re: [PATCH 3/3] arm64: dts: rockchip: Remove UART2 from RGB30
From: Ahmad Fatoum @ 2024-03-30 13:13 UTC (permalink / raw)
To: Chris Morgan, linux-rockchip
Cc: linux-clk, devicetree, sboyd, mturquette, heiko, conor+dt,
krzysztof.kozlowski+dt, robh+dt, Chris Morgan
In-Reply-To: <20231018153357.343142-4-macroalpha82@gmail.com>
Hello Chris,
On 18.10.23 17:33, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> The Powkiddy RGB30 has no onboard UART header, so remove the reference
> to it in the device tree. This was left on by mistake in the initial
> commit.
Do you know if the UART is perhaps available over testpoints?
If yes, having a DT-overlay upstream enabling it along with documentation could be useful.
If not, how do you do low-level debugging on the RBG30 in absence of the serial console?
Thanks,
Ahmad
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
> arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rgb30.dts | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rgb30.dts b/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rgb30.dts
> index 3ebc21608213..1ead3c5c24b3 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rgb30.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rgb30.dts
> @@ -64,6 +64,10 @@ simple-audio-card,cpu {
>
> /delete-node/ &adc_keys;
>
> +&chosen {
> + /delete-property/ stdout-path;
> +};
> +
> &cru {
> assigned-clocks = <&pmucru CLK_RTC_32K>, <&cru PLL_GPLL>,
> <&pmucru PLL_PPLL>, <&cru PLL_VPLL>;
> @@ -149,4 +153,9 @@ rk817_charger: charger {
> };
> };
>
> +/* There is no UART header visible on the board for this device. */
> +&uart2 {
> + status = "disabled";
> +};
> +
> /delete-node/ &vibrator;
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 3/3] dt-bindings: adc: Document XuanTie TH1520 ADC
From: kernel test robot @ 2024-03-30 11:58 UTC (permalink / raw)
To: wefu, jszhang, guoren, conor, robh, krzysztof.kozlowski+dt,
paul.walmsley, palmer, aou, jic23, lars, andriy.shevchenko,
nuno.sa, marcelo.schmitt, bigunclemax, marius.cristea, fr0st61te,
okan.sahin, marcus.folkesson, schnelle, lee, mike.looijmans
Cc: oe-kbuild-all, linux-riscv, devicetree, linux-kernel, linux-iio,
Wei Fu
In-Reply-To: <20240329200241.4122000-4-wefu@redhat.com>
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on robh/for-next linus/master v6.9-rc1 next-20240328]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/wefu-redhat-com/drivers-iio-adc-Add-XuanTie-TH1520-ADC-driver/20240330-041029
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20240329200241.4122000-4-wefu%40redhat.com
patch subject: [PATCH 3/3] dt-bindings: adc: Document XuanTie TH1520 ADC
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240330/202403301900.9wSnTE6y-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403301900.9wSnTE6y-lkp@intel.com/
dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/iio/adc/thead,th1520.yaml:45:1: [error] syntax error: found character '\t' that cannot start any token (syntax)
--
>> Documentation/devicetree/bindings/iio/adc/thead,th1520.yaml:45:1: found a tab character where an indentation space is expected
--
>> Documentation/devicetree/bindings/iio/adc/thead,th1520.yaml: ignoring, error parsing file
vim +45 Documentation/devicetree/bindings/iio/adc/thead,th1520.yaml
41
42 examples:
43 - |
44 adc: adc@0xfffff51000 {
> 45 compatible = "thead,th1520-adc";
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [v1 0/3] Add i.MX8Q HSIO PHY driver support
From: Krzysztof Kozlowski @ 2024-03-30 11:55 UTC (permalink / raw)
To: Richard Zhu, vkoul, kishon, robh+dt, krzysztof.kozlowski+dt,
conor+dt, frank.li
Cc: linux-phy, devicetree, linux-arm-kernel, linux-kernel, kernel,
linux-imx
In-Reply-To: <1711699790-16494-1-git-send-email-hongxing.zhu@nxp.com>
On 29/03/2024 09:09, Richard Zhu wrote:
> v1 changes:
> - Rebase to the 6.9-rc1, and constify of_phandle_args in xlate.
> No other changes.
>
I found some RFC of this... confusing so:
1. v1 is the first version. If you send RFC, that RFC is v1, so anything
newer is v2 or whatever.
2. One patchset per 24h. Give people chance to actually review your code.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: display/msm: sm8150-mdss: add DP node
From: Krzysztof Kozlowski @ 2024-03-30 11:52 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
Vinod Koul
Cc: linux-arm-msm, dri-devel, freedreno, devicetree
In-Reply-To: <966deb7d-847f-451b-8f93-017d703b50c3@linaro.org>
On 27/03/2024 11:11, Krzysztof Kozlowski wrote:
> On 26/03/2024 21:02, Dmitry Baryshkov wrote:
>> As Qualcomm SM8150 got support for the DisplayPort, add displayport@
>> node as a valid child to the MDSS node.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
If there is going to be resend, please switch to "contains" and only
sm8150 compatible just like:
https://lore.kernel.org/all/20240329-sm6350-dp-v2-2-e46dceb32ef5@fairphone.com/
(but no need to resend just for that)
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/3] dt-bindings: display: msm: sm6350-mdss: document DP controller subnode
From: Krzysztof Kozlowski @ 2024-03-30 11:49 UTC (permalink / raw)
To: Luca Weiss, Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kuogee Hsieh,
Krishna Manikandan, Bjorn Andersson, Konrad Dybcio
Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, dri-devel,
freedreno, devicetree, linux-kernel
In-Reply-To: <20240329-sm6350-dp-v2-2-e46dceb32ef5@fairphone.com>
On 29/03/2024 08:45, Luca Weiss wrote:
> Document the displayport controller subnode of the SM6350 MDSS.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 2/5] soc: qcom: llcc: Add regmap for Broadcast_AND region
From: Krzysztof Kozlowski @ 2024-03-30 11:46 UTC (permalink / raw)
To: Unnathi Chalicheemala, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, kernel
In-Reply-To: <20240329-llcc-broadcast-and-v4-2-107c76fd8ceb@quicinc.com>
On 29/03/2024 22:53, Unnathi Chalicheemala wrote:
> Define new regmap structure for Broadcast_AND region and initialize
> this regmap when HW block version is greater than 4.1, otherwise
> initialize as a NULL pointer for backwards compatibility.
>
> + struct regmap *regmap;
> u32 act_ctrl_reg;
> u32 act_clear_reg;
> u32 status_reg;
> @@ -849,7 +850,8 @@ static int llcc_update_act_ctrl(u32 sid,
> return ret;
>
> if (drv_data->version >= LLCC_VERSION_4_1_0_0) {
> - ret = regmap_read_poll_timeout(drv_data->bcast_regmap, status_reg,
> + regmap = drv_data->bcast_and_regmap ?: drv_data->bcast_regmap;
> + ret = regmap_read_poll_timeout(regmap, status_reg,
> slice_status, (slice_status & ACT_COMPLETE),
> 0, LLCC_STATUS_READ_DELAY);
> if (ret)
> @@ -1284,6 +1286,16 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>
> drv_data->version = version;
>
> + /* Applicable only when drv_data->version >= 4.1 */
> + drv_data->bcast_and_regmap = qcom_llcc_init_mmio(pdev, i + 1, "llcc_broadcast_and_base");
> + if (IS_ERR(drv_data->bcast_and_regmap)) {
I am pretty sure this breaks all users. Can you please explain how do
you maintain ABI and that IS_ERR() is applied only for version >= 4.1?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 1/5] dt-bindings: arm: msm: Add llcc Broadcast_AND register
From: Krzysztof Kozlowski @ 2024-03-30 11:44 UTC (permalink / raw)
To: Unnathi Chalicheemala, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, kernel
In-Reply-To: <20240329-llcc-broadcast-and-v4-1-107c76fd8ceb@quicinc.com>
On 29/03/2024 22:53, Unnathi Chalicheemala wrote:
> The LLCC block in SM8450, SM8550 and SM8650 have a new register
> space for Broadcast_AND region. This is used to check that all
> channels have bit set to "1", mainly in SCID activation/deactivation.
>
> Previously we were mapping only the Broadcast_OR region assuming
> there was only one broadcast register region. Now we also map
> Broadcast_AND region.
>
> Signed-off-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.
https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 0/5] LLCC: Support for Broadcast_AND region
From: Krzysztof Kozlowski @ 2024-03-30 11:42 UTC (permalink / raw)
To: Unnathi Chalicheemala, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, kernel
In-Reply-To: <20240329-llcc-broadcast-and-v4-0-107c76fd8ceb@quicinc.com>
On 29/03/2024 22:53, Unnathi Chalicheemala wrote:
> This series adds:
> 1. Device tree register mapping for Broadcast_AND region in SM8450,
> SM8550, SM8650.
> 2. LLCC driver updates to reflect addition of Broadcast_AND regmap.
>
> To support CSR programming, a broadcast interface is used to program all
> channels in a single command. Until SM8450 there was only one broadcast
> region (Broadcast_OR) used to broadcast write and check for status bit
> 0. From SM8450 onwards another broadcast region (Broadcast_AND) has been
> added which checks for status bit 1.
>
> This series updates the device trees from SM8450 onwards to have a
> mapping to this Broadcast_AND region. It also updates the llcc_drv_data
> structure with a regmap for Broadcast_AND region and corrects the
> broadcast region used to check for status bit 1.
Your way of sending patches makes it difficult for us to review them.
b4 diff -C '<20240329-llcc-broadcast-and-v4-0-107c76fd8ceb@quicinc.com>'
Grabbing thread from
lore.kernel.org/all/20240329-llcc-broadcast-and-v4-0-107c76fd8ceb@quicinc.com/t.mbox.gz
Checking for older revisions
Added from v3: 5 patches
---
Analyzing 39 messages in the thread
Preparing fake-am for v3: dt-bindings: arm: msm: Add llcc Broadcast_AND
register
ERROR: v3 series incomplete; unable to create a fake-am range
---
Could not create fake-am range for lower series v3
Please reach internally within Qualcomm to get some guidance how to
properly set up your work environment.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 5/5] cadence-xspi: Add xfer capabilities
From: Krzysztof Kozlowski @ 2024-03-30 11:37 UTC (permalink / raw)
To: Witold Sadowski, linux-kernel, linux-spi, devicetree
Cc: broonie, robh, krzysztof.kozlowski+dt, conor+dt, pthombar
In-Reply-To: <20240329194849.25554-6-wsadowski@marvell.com>
On 29/03/2024 20:48, Witold Sadowski wrote:
> Add support for iMRVL xfer hw_overlay of Cadence xSPI
> block.
> MRVL Xfer overlay extend xSPI capabilities, to support
> non-memory SPI operations.
> With generic xSPI command it allows to create any
> required SPI transaction
Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.
Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
... and build your code because this does not compile. :(
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 4/5] driver: spi: cadence: Add ACPI support
From: Krzysztof Kozlowski @ 2024-03-30 11:36 UTC (permalink / raw)
To: Witold Sadowski, linux-kernel, linux-spi, devicetree
Cc: broonie, robh, krzysztof.kozlowski+dt, conor+dt, pthombar,
Piyush Malgujar
In-Reply-To: <20240329194849.25554-5-wsadowski@marvell.com>
On 29/03/2024 20:48, Witold Sadowski wrote:
> From: Piyush Malgujar <pmalgujar@marvell.com>
>
> These changes enables to read the configs from ACPI tables as
> required for successful probing in ACPI uefi environment.
> In case of ACPI disabled/dts based environment, it will continue to
> read configs from dts as before.
>
Random subjects... Please use subject prefixes matching the subsystem.
You can get them for example with `git log --oneline --
DIRECTORY_OR_FILE` on the directory your patch is touching.
> }
>
> +static const struct acpi_device_id cdns_xspi_acpi_match[] = {
> + {"cdns,xspi-nor", 0},
> + {"mrvl,xspi-nor", 0},
How is this ACPI?
> + {},
> +};
> +MODULE_DEVICE_TABLE(acpi, cdns_xspi_acpi_match);
> +#ifdef CONFIG_OF
This was never compiled. I could understand not testing bindings,
because it is something relatively new - like 5 or 6 years. But not
compiling code is less understandable.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/5] spi: cadence: Add Marvell IP modification changes
From: Krzysztof Kozlowski @ 2024-03-30 11:33 UTC (permalink / raw)
To: Witold Sadowski, linux-kernel, linux-spi, devicetree
Cc: broonie, robh, krzysztof.kozlowski+dt, conor+dt, pthombar
In-Reply-To: <20240329194849.25554-3-wsadowski@marvell.com>
On 29/03/2024 20:48, Witold Sadowski wrote:
> Add support for Marvell IP modification - clock divider,
> and PHY config, and IRQ clearing.
> Clock divider block is build into Cadence XSPI controller
> and is connected directly to 800MHz clock.
> As PHY config is not set directly in IP block, driver can
> load custom PHY configuration values.
> To correctly clear interrupt in Marvell implementation
> MSI-X must be cleared too.
Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>
> Signed-off-by: Witold Sadowski <wsadowski@marvell.com>
> ---
> +
> +static bool cdns_xspi_get_hw_overlay(struct platform_device *pdev)
> +{
> + int err;
> +
> + err = device_property_match_string(&pdev->dev,
> + "compatible", "mrvl,xspi-nor");
No, do not add matching in some random parts of the code, but use driver
match/data from ID table.
....
>
> + cdns_xspi_print_phy_config(cdns_xspi);
> ret = cdns_xspi_controller_init(cdns_xspi);
> if (ret) {
> dev_err(dev, "Failed to initialize controller\n");
> @@ -613,6 +911,9 @@ static const struct of_device_id cdns_xspi_of_match[] = {
> {
> .compatible = "cdns,xspi-nor",
> },
> + {
> + .compatible = "mrvl,xspi-nor",
This falsely suggest they are compatible :/
> + },
> { /* end of table */}
> };
> MODULE_DEVICE_TABLE(of, cdns_xspi_of_match);
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/5] spi: cadence: Add new bindings documentation for Cadence XSPI
From: Krzysztof Kozlowski @ 2024-03-30 11:32 UTC (permalink / raw)
To: Witold Sadowski, linux-kernel, linux-spi, devicetree
Cc: broonie, robh, krzysztof.kozlowski+dt, conor+dt, pthombar
In-Reply-To: <20240329194849.25554-2-wsadowski@marvell.com>
On 29/03/2024 20:48, Witold Sadowski wrote:
> Add new bindings:
> - mrvl,xspi-nor compatible string
> Compatible string to enable Marvell XSPI modification
> - Multiple PHY configuration registers
> - base for xfer register set
>
> Signed-off-by: Witold Sadowski <wsadowski@marvell.com>
Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.
> ---
> .../devicetree/bindings/spi/cdns,xspi.yaml | 84 ++++++++++++++++++-
> 1 file changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> index eb0f92468185..d1fde8d4e9b8 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> @@ -20,23 +20,74 @@ allOf:
>
> properties:
> compatible:
> - const: cdns,xspi-nor
> + - const: cdns,xspi-nor
> + - const: mrvl,xspi-nor
It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.
There is a lot of things happening here, but I won't perform review if
the code was never tested. Sorry, please test before sending.
Best regards,
Krzysztof
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox