* 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
* [PATCH 1/9] arm64: dts: rockchip: Add cpu regulators and vcc5v0_sys to Khadas Edge 2
From: efectn @ 2024-03-30 11:25 UTC (permalink / raw)
To: heiko
Cc: conor+dt, devicetree, efectn, krzysztof.kozlowski+dt,
linux-arm-kernel, linux-kernel, linux-rockchip, robh+dt,
sebastian.reichel
In-Reply-To: <5a7bd2cd8703e51382abfc11242de59d45286477.1708381247.git.efectn@protonmail.com>
Hi Heiko,
Sorry if i bother you. Can you review the series? If there is a problem i will send the v2.
^ permalink raw reply
* Re: [PATCH v5 4/5] clk: qcom: ipq9574: Use icc-clk for enabling NoC related clocks
From: Varadarajan Narayanan @ 2024-03-30 11:17 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Stephen Boyd, andersson, conor+dt, devicetree, djakov,
dmitry.baryshkov, konrad.dybcio, krzysztof.kozlowski+dt,
linux-arm-msm, linux-clk, linux-kernel, linux-pm, mturquette,
quic_anusha, robh
In-Reply-To: <5570c921-0103-4e92-be9a-da9c1b7cbd79@linaro.org>
On Sat, Mar 30, 2024 at 11:28:09AM +0100, Krzysztof Kozlowski wrote:
> On 30/03/2024 10:30, Varadarajan Narayanan wrote:
> > On Fri, Mar 29, 2024 at 01:10:03PM +0100, Krzysztof Kozlowski wrote:
> >> On 29/03/2024 11:55, Varadarajan Narayanan wrote:
> >>>>> +
> >>>>> +enum {
> >>>>> + ICC_ANOC_PCIE0,
> >>>>> + ICC_SNOC_PCIE0,
> >>>>> + ICC_ANOC_PCIE1,
> >>>>> + ICC_SNOC_PCIE1,
> >>>>> + ICC_ANOC_PCIE2,
> >>>>> + ICC_SNOC_PCIE2,
> >>>>> + ICC_ANOC_PCIE3,
> >>>>> + ICC_SNOC_PCIE3,
> >>>>> + ICC_SNOC_USB,
> >>>>> + ICC_ANOC_USB_AXI,
> >>>>> + ICC_NSSNOC_NSSCC,
> >>>>> + ICC_NSSNOC_SNOC_0,
> >>>>> + ICC_NSSNOC_SNOC_1,
> >>>>> + ICC_NSSNOC_PCNOC_1,
> >>>>> + ICC_NSSNOC_QOSGEN_REF,
> >>>>> + ICC_NSSNOC_TIMEOUT_REF,
> >>>>> + ICC_NSSNOC_XO_DCD,
> >>>>> + ICC_NSSNOC_ATB,
> >>>>> + ICC_MEM_NOC_NSSNOC,
> >>>>> + ICC_NSSNOC_MEMNOC,
> >>>>> + ICC_NSSNOC_MEM_NOC_1,
> >>>>> +};
> >>>>
> >>>> Are these supposed to be in a dt-binding header?
> >>>
> >>> Since these don't directly relate to the ids in the dt-bindings
> >>> not sure if they will be permitted there. Will move and post a
> >>> new version and get feedback.
> >>
> >> You can answer this by yourself by looking at your DTS. Do you use them
> >> as well in the DTS?
> >
> > I can use them in the DTS. The icc-clk framework automatically
> > creates master and slave nodes as 'n' and 'n+1'. Hence I can have
> > something like this in the dt-bindings include file
> >
> > #define ICC_ANOC_PCIE0 0
> > #define ICC_SNOC_PCIE0 1
> > .
> > .
> > .
> > #define ICC_NSSNOC_MEM_NOC_1 20
> >
> > #define MASTER(x) ((ICC_ ## x) * 2)
> > #define SLAVE(x) (MASTER(x) + 1)
>
> I don't understand this or maybe I misunderstood the purpose of this
> define. It does not matter if you "can" use something in DT. The
> question is: do you use them.
Yes. It will be used fot the pcie nodes. These defines are for
specifying the endpoints. The icc driver identifies a path that
can connect these endpoints. The peripheral drivers' DT nodes
will make use of these defines.
> >> It's a pity we see here only parts of DTS, instead of full interconnect
> >> usage.
> >
> > Unfortunately cannot include the pcie dts changes with this
> > patch, but you can refer to them at https://lore.kernel.org/linux-arm-msm/20230519090219.15925-5-quic_devipriy@quicinc.com/
> >
> > The above macros will be used in the pcie node as follows
> >
> > pcie0: pci@28000000 {
> > compatible = "qcom,pcie-ipq9574";
> > . . .
> > interconnects = <&gcc MASTER(ANOC_PCIE0) &gcc SLAVE(ANOC_PCIE0)>,
> > <&gcc MASTER(SNOC_PCIE0) &gcc SLAVE(SNOC_PCIE0)>;
> > interconnect-names = "pcie-mem", "cpu-pcie";
>
> Then why did you add header which is not used?
Since they are a part of the interconnect driver. The peripherals
that use the interconnects will make use of them to specify the
endpoints. After changing per Boyd's comments, the header will
be used from gcc driver. Will post a new version. Kindly review
that.
Thanks
Varada
> I will respond there...
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: pm6150: correct USB VBUS regulator compatible
From: Konrad Dybcio @ 2024-03-30 10:49 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bjorn Andersson, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Dmitry Baryshkov,
Greg Kroah-Hartman, Danila Tikhonov, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <20240330091311.6224-2-krzysztof.kozlowski@linaro.org>
On 30.03.2024 10:13 AM, Krzysztof Kozlowski wrote:
> The first part of the compatible of USB VBUS node misses ending quote,
> thus we have one long compatible consisting of two compatible strings
> leading to dtbs_check warnings:
>
> sc7180-idp.dtb: usb-vbus-regulator@1100: compatible:0: 'qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg' does not match '^[a-zA-Z0-9][a-zA-Z0-9,+\\-._/]+$'
> sc7180-idp.dtb: /soc@0/spmi@c440000/pmic@0/usb-vbus-regulator@1100: failed to match any schema with compatible: ['qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg']
>
> Reported-by: Rob Herring <robh@kernel.org>
> Fixes: f81c2f01cad6 ("arm64: dts: qcom: pm6150: define USB-C related blocks")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Konrad
^ permalink raw reply
* Re: [PATCH v2 1/2] arm64: dts: qcom: pm6150: correct Type-C compatible
From: Konrad Dybcio @ 2024-03-30 10:48 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bjorn Andersson, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Dmitry Baryshkov,
Greg Kroah-Hartman, Danila Tikhonov, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <20240330091311.6224-1-krzysztof.kozlowski@linaro.org>
On 30.03.2024 10:13 AM, Krzysztof Kozlowski wrote:
> The first part of the compatible of Type-C node misses ending quote,
> thus we have one long compatible consisting of two compatible strings
> leading to dtbs_check warnings:
>
> sc7180-acer-aspire1.dtb: pmic@0: typec@1500:compatible: 'oneOf' conditional failed, one must be fixed
> sc7180-acer-aspire1.dtb: typec@1500: compatible:0: 'qcom,pm6150-typec,\n qcom,pm8150b-typec' does not match '^[a-zA-Z0-9][a-zA-Z0-9,+\\-._/]+$'
>
> Reported-by: Rob Herring <robh@kernel.org>
> Fixes: f81c2f01cad6 ("arm64: dts: qcom: pm6150: define USB-C related blocks")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> ---
LOL!
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Konrad
^ permalink raw reply
* Re: [PATCH 3/3] drm: panel: Add LG sw43408 panel driver
From: Marijn Suijten @ 2024-03-30 10:27 UTC (permalink / raw)
To: Dmitry Baryshkov
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: <20240330-lg-sw43408-panel-v1-3-f5580fc9f2da@linaro.org>
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?
> + 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)?
> +
> + 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?
> + 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?
> +
> + 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?
- 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
>
^ permalink raw reply
* Re: [PATCH v5 1/5] dt-bindings: interconnect: Add Qualcomm IPQ9574 support
From: Krzysztof Kozlowski @ 2024-03-30 10:30 UTC (permalink / raw)
To: Varadarajan Narayanan, andersson, konrad.dybcio, mturquette,
sboyd, robh, krzysztof.kozlowski+dt, conor+dt, djakov,
dmitry.baryshkov, quic_anusha, linux-arm-msm, linux-clk,
devicetree, linux-kernel, linux-pm
In-Reply-To: <20240328075936.223461-2-quic_varada@quicinc.com>
On 28/03/2024 08:59, Varadarajan Narayanan wrote:
> Add interconnect-cells to clock provider so that it can be
> used as icc provider.
>
> Add master/slave ids for Qualcomm IPQ9574 Network-On-Chip
> interfaces. This will be used by the gcc-ipq9574 driver
> that will for providing interconnect services using the
> icc-clk framework.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Please remove my Reviewed tag.
It seems you do not use this binding header: neither in your DTS and nor
in the driver.
Consider the patch NAK-ed and send new version which removes unused header.
Please provide link to upstreamed DTS using this, so we can validate
your usage. Otherwise it looks just wrong and you try to upstream
something which will not pass dtbs checks on the first day, for example.
NAK
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v5 4/5] clk: qcom: ipq9574: Use icc-clk for enabling NoC related clocks
From: Krzysztof Kozlowski @ 2024-03-30 10:28 UTC (permalink / raw)
To: Varadarajan Narayanan
Cc: Stephen Boyd, andersson, conor+dt, devicetree, djakov,
dmitry.baryshkov, konrad.dybcio, krzysztof.kozlowski+dt,
linux-arm-msm, linux-clk, linux-kernel, linux-pm, mturquette,
quic_anusha, robh
In-Reply-To: <Zgfbs5SFN2cA0gSK@hu-varada-blr.qualcomm.com>
On 30/03/2024 10:30, Varadarajan Narayanan wrote:
> On Fri, Mar 29, 2024 at 01:10:03PM +0100, Krzysztof Kozlowski wrote:
>> On 29/03/2024 11:55, Varadarajan Narayanan wrote:
>>>>> +
>>>>> +enum {
>>>>> + ICC_ANOC_PCIE0,
>>>>> + ICC_SNOC_PCIE0,
>>>>> + ICC_ANOC_PCIE1,
>>>>> + ICC_SNOC_PCIE1,
>>>>> + ICC_ANOC_PCIE2,
>>>>> + ICC_SNOC_PCIE2,
>>>>> + ICC_ANOC_PCIE3,
>>>>> + ICC_SNOC_PCIE3,
>>>>> + ICC_SNOC_USB,
>>>>> + ICC_ANOC_USB_AXI,
>>>>> + ICC_NSSNOC_NSSCC,
>>>>> + ICC_NSSNOC_SNOC_0,
>>>>> + ICC_NSSNOC_SNOC_1,
>>>>> + ICC_NSSNOC_PCNOC_1,
>>>>> + ICC_NSSNOC_QOSGEN_REF,
>>>>> + ICC_NSSNOC_TIMEOUT_REF,
>>>>> + ICC_NSSNOC_XO_DCD,
>>>>> + ICC_NSSNOC_ATB,
>>>>> + ICC_MEM_NOC_NSSNOC,
>>>>> + ICC_NSSNOC_MEMNOC,
>>>>> + ICC_NSSNOC_MEM_NOC_1,
>>>>> +};
>>>>
>>>> Are these supposed to be in a dt-binding header?
>>>
>>> Since these don't directly relate to the ids in the dt-bindings
>>> not sure if they will be permitted there. Will move and post a
>>> new version and get feedback.
>>
>> You can answer this by yourself by looking at your DTS. Do you use them
>> as well in the DTS?
>
> I can use them in the DTS. The icc-clk framework automatically
> creates master and slave nodes as 'n' and 'n+1'. Hence I can have
> something like this in the dt-bindings include file
>
> #define ICC_ANOC_PCIE0 0
> #define ICC_SNOC_PCIE0 1
> .
> .
> .
> #define ICC_NSSNOC_MEM_NOC_1 20
>
> #define MASTER(x) ((ICC_ ## x) * 2)
> #define SLAVE(x) (MASTER(x) + 1)
I don't understand this or maybe I misunderstood the purpose of this
define. It does not matter if you "can" use something in DT. The
question is: do you use them.
>
>> It's a pity we see here only parts of DTS, instead of full interconnect
>> usage.
>
> Unfortunately cannot include the pcie dts changes with this
> patch, but you can refer to them at https://lore.kernel.org/linux-arm-msm/20230519090219.15925-5-quic_devipriy@quicinc.com/
>
> The above macros will be used in the pcie node as follows
>
> pcie0: pci@28000000 {
> compatible = "qcom,pcie-ipq9574";
> . . .
> interconnects = <&gcc MASTER(ANOC_PCIE0) &gcc SLAVE(ANOC_PCIE0)>,
> <&gcc MASTER(SNOC_PCIE0) &gcc SLAVE(SNOC_PCIE0)>;
> interconnect-names = "pcie-mem", "cpu-pcie";
Then why did you add header which is not used?
I will respond there...
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH 1/2] arm64: dts: rockchip: Enable gpu on Cool Pi CM5
From: Andy Yan @ 2024-03-30 10:01 UTC (permalink / raw)
To: heiko
Cc: krzysztof.kozlowski+dt, devicetree, dsimic, conor+dt,
linux-arm-kernel, linux-kernel, linux-rockchip, sebastian.reichel,
Andy Yan
Enable mali gpu node and add the board specific supply-regulator.
Signed-off-by: Andy Yan <andyshrk@163.com>
---
arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5.dtsi
index 94ecb9b4f98f..6f5cf6a06c2c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5.dtsi
@@ -136,6 +136,11 @@ &gmac0_rgmii_clk
status = "okay";
};
+&gpu {
+ mali-supply = <&vdd_gpu_s0>;
+ status = "okay";
+};
+
&i2c0 {
pinctrl-0 = <&i2c0m2_xfer>;
status = "okay";
--
2.34.1
^ permalink raw reply related
* [PATCH 2/2] arm64: dts: rockchip: Enable gpu on Cool Pi 4B
From: Andy Yan @ 2024-03-30 10:01 UTC (permalink / raw)
To: heiko
Cc: krzysztof.kozlowski+dt, devicetree, dsimic, conor+dt,
linux-arm-kernel, linux-kernel, linux-rockchip, sebastian.reichel,
Andy Yan
In-Reply-To: <20240330100134.3588223-1-andyshrk@163.com>
Enable mali gpu node and add the board specific supply-regulator.
Signed-off-by: Andy Yan <andyshrk@163.com>
---
arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts b/arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts
index e037bf9db75a..25a2ae7d4827 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts
@@ -203,6 +203,11 @@ &cpu_b2 {
cpu-supply = <&vdd_cpu_big1_s0>;
};
+&gpu {
+ mali-supply = <&vdd_gpu_s0>;
+ status = "okay";
+};
+
&i2c0 {
pinctrl-0 = <&i2c0m2_xfer>;
status = "okay";
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v5 4/5] clk: qcom: ipq9574: Use icc-clk for enabling NoC related clocks
From: Varadarajan Narayanan @ 2024-03-30 9:30 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Stephen Boyd, andersson, conor+dt, devicetree, djakov,
dmitry.baryshkov, konrad.dybcio, krzysztof.kozlowski+dt,
linux-arm-msm, linux-clk, linux-kernel, linux-pm, mturquette,
quic_anusha, robh
In-Reply-To: <031d0a35-b192-4161-beef-97b89d5d1da6@linaro.org>
On Fri, Mar 29, 2024 at 01:10:03PM +0100, Krzysztof Kozlowski wrote:
> On 29/03/2024 11:55, Varadarajan Narayanan wrote:
> >>> +
> >>> +enum {
> >>> + ICC_ANOC_PCIE0,
> >>> + ICC_SNOC_PCIE0,
> >>> + ICC_ANOC_PCIE1,
> >>> + ICC_SNOC_PCIE1,
> >>> + ICC_ANOC_PCIE2,
> >>> + ICC_SNOC_PCIE2,
> >>> + ICC_ANOC_PCIE3,
> >>> + ICC_SNOC_PCIE3,
> >>> + ICC_SNOC_USB,
> >>> + ICC_ANOC_USB_AXI,
> >>> + ICC_NSSNOC_NSSCC,
> >>> + ICC_NSSNOC_SNOC_0,
> >>> + ICC_NSSNOC_SNOC_1,
> >>> + ICC_NSSNOC_PCNOC_1,
> >>> + ICC_NSSNOC_QOSGEN_REF,
> >>> + ICC_NSSNOC_TIMEOUT_REF,
> >>> + ICC_NSSNOC_XO_DCD,
> >>> + ICC_NSSNOC_ATB,
> >>> + ICC_MEM_NOC_NSSNOC,
> >>> + ICC_NSSNOC_MEMNOC,
> >>> + ICC_NSSNOC_MEM_NOC_1,
> >>> +};
> >>
> >> Are these supposed to be in a dt-binding header?
> >
> > Since these don't directly relate to the ids in the dt-bindings
> > not sure if they will be permitted there. Will move and post a
> > new version and get feedback.
>
> You can answer this by yourself by looking at your DTS. Do you use them
> as well in the DTS?
I can use them in the DTS. The icc-clk framework automatically
creates master and slave nodes as 'n' and 'n+1'. Hence I can have
something like this in the dt-bindings include file
#define ICC_ANOC_PCIE0 0
#define ICC_SNOC_PCIE0 1
.
.
.
#define ICC_NSSNOC_MEM_NOC_1 20
#define MASTER(x) ((ICC_ ## x) * 2)
#define SLAVE(x) (MASTER(x) + 1)
> It's a pity we see here only parts of DTS, instead of full interconnect
> usage.
Unfortunately cannot include the pcie dts changes with this
patch, but you can refer to them at https://lore.kernel.org/linux-arm-msm/20230519090219.15925-5-quic_devipriy@quicinc.com/
The above macros will be used in the pcie node as follows
pcie0: pci@28000000 {
compatible = "qcom,pcie-ipq9574";
. . .
interconnects = <&gcc MASTER(ANOC_PCIE0) &gcc SLAVE(ANOC_PCIE0)>,
<&gcc MASTER(SNOC_PCIE0) &gcc SLAVE(SNOC_PCIE0)>;
interconnect-names = "pcie-mem", "cpu-pcie";
. . .
};
Hope this is acceptable.
Thanks
Varada
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: mfd: x-powers,axp152: add boost regulator
From: Krzysztof Kozlowski @ 2024-03-30 9:30 UTC (permalink / raw)
To: Andre Przywara, Chen-Yu Tsai, Lee Jones, Liam Girdwood,
Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: devicetree, linux-kernel, linux-sunxi, Jernej Skrabec,
Samuel Holland, Ryan Walklin, Chris Morgan
In-Reply-To: <20240329235033.25309-3-andre.przywara@arm.com>
On 30/03/2024 00:50, Andre Przywara wrote:
> The X-Powers AXP717 contains a boost regulator, that it meant to provide
> the 5V USB VBUS voltage when the devices operates on battery.
>
> Add the name "boost" to the regexp describing the allowed node names,
> to allow the regulator to be described in the devicetree.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> index b8e8db0d58e9c..14ab367fc8871 100644
> --- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> +++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> @@ -274,7 +274,7 @@ properties:
> Defines the work frequency of DC-DC in kHz.
>
> patternProperties:
> - "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|cpusldo|drivevbus|dc5ldo)$":
> + "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|cpusldo|drivevbus|dc5ldo|boost)$":
That's not an easy to read regex...
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
If driver does not depend on _, please consider dropping (_|-).
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG bindings
From: Krzysztof Kozlowski @ 2024-03-30 9:27 UTC (permalink / raw)
To: Klymenko, Anatoliy, Conor Dooley
Cc: Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Simek, Michal,
Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman,
Jernej Skrabec, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab, Tomi Valkeinen,
dri-devel@lists.freedesktop.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-media@vger.kernel.org
In-Reply-To: <MW4PR12MB7165E47719C72CD5CEB68218E6392@MW4PR12MB7165.namprd12.prod.outlook.com>
On 30/03/2024 03:02, Klymenko, Anatoliy wrote:
>>>>
>>>
>>> Sorry, I didn't explain properly what I'm trying to achieve. I need to
>>> create a DT node property that represents video signal format, one of
>>> MEDIA_BUS_FMT_* from include/uapi/linux/media-bus-format.h. It would
>> be
>>> nice to reuse the same symbolic values in the device tree. What is the
>>> best approach here? Should I create a separate header in
There is no user of this new header, so I don't agree. Please send
either full work or link your other upstreamed patchset. Anything sent
as "DO NOT MERGE" does not count because it is not an user.
Without the DTS user I claim that you do not bind here anything...
>>> include/dt-bindings with the same or similar (to avoid multiple
>>> definition errors) defines, or is it better to create a symlink to
>>> media-bus-format.h like include/dt-bindings/linux-event-codes.h?
Copying or symlinking entire header into bindings does not help us to
understand what is exactly a binding here.
For example, maybe you encode runtime information into DT (don't do
this) and that's why you need these defines... Or maybe your block has
some capabilities. Dunno, patch was not tested, is defined as do not
merge and is not explaining any of these.
Therefore, please provide complete set of users ready to be merged, test
your patches, provide rationale why this is supposed to be a binding and
why do you think it represents hardware configuration, not OS policy or
runtime configuration.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
From: Krzysztof Kozlowski @ 2024-03-30 9:19 UTC (permalink / raw)
To: Dmitry Baryshkov, 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-v1-1-f5580fc9f2da@linaro.org>
On 30/03/2024 04:59, Dmitry Baryshkov wrote:
> From: Sumit Semwal <sumit.semwal@linaro.org>
>
> LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel present on Google Pixel 3
> phones.
... Few nits, which could be ignored, and one thing which is blocking -
missing example.
> +
> +maintainers:
> + - Caleb Connolly <caleb.connolly@linaro.org>
> +
> +description: |
Do not need '|' unless you need to preserve formatting.
> + 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
> +
> +additionalProperties: false
This goes after required:
> +
> +required:
> + - compatible
> + - vddi-supply
> + - vpnl-supply
> + - reset-gpios
Please add example DTS.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 17/18] dt-bindings: pci: rockchip,rk3399-pcie-ep: Add ep-gpios property
From: Krzysztof Kozlowski @ 2024-03-30 9:16 UTC (permalink / raw)
To: Damien Le Moal, Manivannan Sadhasivam, Lorenzo Pieralisi,
Kishon Vijay Abraham I, Shawn Lin, Krzysztof Wilczyński,
Bjorn Helgaas, Heiko Stuebner, linux-pci, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-18-dlemoal@kernel.org>
On 30/03/2024 05:19, Damien Le Moal wrote:
> From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>
> Describe the `ep-gpios` property which is used to map the PERST# input
> signal for endpoint mode.
>
> Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
> .../devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
> index 6b62f6f58efe..9331d44d6963 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
> @@ -30,6 +30,9 @@ properties:
> maximum: 32
> default: 32
>
> + ep-gpios:
> + description: Input GPIO configured for the PERST# signal.
Missing maxItems. But more important: why existing property perst-gpios,
which you already have there in common schema, is not correct for this case?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [RFC PATCH 0/2] Add gpio-usb-c-connector compatible
From: Krishna Kurapati PSSNV @ 2024-03-30 9:13 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: <CAA8EJpqx+VFW8z6oG=+pnhPN97Q3R6z+ygf85Uspve-9syQsUw@mail.gmail.com>
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.
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.
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.
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
* [PATCH v2 2/2] arm64: dts: qcom: pm6150: correct USB VBUS regulator compatible
From: Krzysztof Kozlowski @ 2024-03-30 9:13 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Dmitry Baryshkov, Greg Kroah-Hartman,
Danila Tikhonov, linux-arm-msm, devicetree, linux-kernel
Cc: Krzysztof Kozlowski
In-Reply-To: <20240330091311.6224-1-krzysztof.kozlowski@linaro.org>
The first part of the compatible of USB VBUS node misses ending quote,
thus we have one long compatible consisting of two compatible strings
leading to dtbs_check warnings:
sc7180-idp.dtb: usb-vbus-regulator@1100: compatible:0: 'qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg' does not match '^[a-zA-Z0-9][a-zA-Z0-9,+\\-._/]+$'
sc7180-idp.dtb: /soc@0/spmi@c440000/pmic@0/usb-vbus-regulator@1100: failed to match any schema with compatible: ['qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg']
Reported-by: Rob Herring <robh@kernel.org>
Fixes: f81c2f01cad6 ("arm64: dts: qcom: pm6150: define USB-C related blocks")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. New patch, pointed out by: Gabor Juhos
---
arch/arm64/boot/dts/qcom/pm6150.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/pm6150.dtsi b/arch/arm64/boot/dts/qcom/pm6150.dtsi
index b20a639cddf3..6de6ed562d97 100644
--- a/arch/arm64/boot/dts/qcom/pm6150.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm6150.dtsi
@@ -64,8 +64,8 @@ pm6150_resin: resin {
};
pm6150_vbus: usb-vbus-regulator@1100 {
- compatible = "qcom,pm6150-vbus-reg,
- qcom,pm8150b-vbus-reg";
+ compatible = "qcom,pm6150-vbus-reg",
+ "qcom,pm8150b-vbus-reg";
reg = <0x1100>;
status = "disabled";
};
--
2.34.1
^ permalink raw reply related
* [PATCH v2 1/2] arm64: dts: qcom: pm6150: correct Type-C compatible
From: Krzysztof Kozlowski @ 2024-03-30 9:13 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Dmitry Baryshkov, Greg Kroah-Hartman,
Danila Tikhonov, linux-arm-msm, devicetree, linux-kernel
Cc: Krzysztof Kozlowski
The first part of the compatible of Type-C node misses ending quote,
thus we have one long compatible consisting of two compatible strings
leading to dtbs_check warnings:
sc7180-acer-aspire1.dtb: pmic@0: typec@1500:compatible: 'oneOf' conditional failed, one must be fixed
sc7180-acer-aspire1.dtb: typec@1500: compatible:0: 'qcom,pm6150-typec,\n qcom,pm8150b-typec' does not match '^[a-zA-Z0-9][a-zA-Z0-9,+\\-._/]+$'
Reported-by: Rob Herring <robh@kernel.org>
Fixes: f81c2f01cad6 ("arm64: dts: qcom: pm6150: define USB-C related blocks")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. Correct warning msg
---
arch/arm64/boot/dts/qcom/pm6150.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/pm6150.dtsi b/arch/arm64/boot/dts/qcom/pm6150.dtsi
index 11158c2bd524..b20a639cddf3 100644
--- a/arch/arm64/boot/dts/qcom/pm6150.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm6150.dtsi
@@ -71,8 +71,8 @@ pm6150_vbus: usb-vbus-regulator@1100 {
};
pm6150_typec: typec@1500 {
- compatible = "qcom,pm6150-typec,
- qcom,pm8150b-typec";
+ compatible = "qcom,pm6150-typec",
+ "qcom,pm8150b-typec";
reg = <0x1500>, <0x1700>;
interrupts = <0x0 0x15 0x00 IRQ_TYPE_EDGE_RISING>,
<0x0 0x15 0x01 IRQ_TYPE_EDGE_BOTH>,
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] arm64: dts: qcom: pm6150: correct Type-C compatible
From: Krzysztof Kozlowski @ 2024-03-30 9:05 UTC (permalink / raw)
To: Gabor Juhos, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
Dmitry Baryshkov, Danila Tikhonov, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <7f28584c-0e9a-44bf-b0c1-7f7a2acfb847@gmail.com>
On 30/03/2024 08:07, Gabor Juhos wrote:
> 2024. 03. 28. 8:45 keltezéssel, Krzysztof Kozlowski írta:
>> The first part of the compatible of Type-C node misses ending quote,
>> thus we have one long compatible consisting of two compatible strings
>> leading to dtbs_check warnings:
>>
>> sc7180-idp.dtb: usb-vbus-regulator@1100: compatible:0: 'qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg' does not match '^[a-zA-Z0-9][a-zA-Z0-9,+\\-._/]+$'
>> sc7180-idp.dtb: /soc@0/spmi@c440000/pmic@0/usb-vbus-regulator@1100: failed to match any schema with compatible: ['qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg']
>>
>> Reported-by: Rob Herring <robh@kernel.org>
>> Fixes: f81c2f01cad6 ("arm64: dts: qcom: pm6150: define USB-C related blocks")
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>> arch/arm64/boot/dts/qcom/pm6150.dtsi | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/pm6150.dtsi b/arch/arm64/boot/dts/qcom/pm6150.dtsi
>> index 11158c2bd524..b20a639cddf3 100644
>> --- a/arch/arm64/boot/dts/qcom/pm6150.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/pm6150.dtsi
>> @@ -71,8 +71,8 @@ pm6150_vbus: usb-vbus-regulator@1100 {
>> };
>>
>> pm6150_typec: typec@1500 {
>> - compatible = "qcom,pm6150-typec,
>> - qcom,pm8150b-typec";
>> + compatible = "qcom,pm6150-typec",
>> + "qcom,pm8150b-typec";
>
> The warning in the commit description is related to the
> 'usb-vbus-regulator@1100' node, whereas this fixes the compatible of a different
> one.
>
> Did you want to fix both?
D'oh! There are two of them! Thanks for the catch. Yes, I want to fix
both of them :)
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: pm6150: correct Type-C compatible
From: Gabor Juhos @ 2024-03-30 7:07 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
Dmitry Baryshkov, Danila Tikhonov, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <20240328074544.5076-1-krzysztof.kozlowski@linaro.org>
2024. 03. 28. 8:45 keltezéssel, Krzysztof Kozlowski írta:
> The first part of the compatible of Type-C node misses ending quote,
> thus we have one long compatible consisting of two compatible strings
> leading to dtbs_check warnings:
>
> sc7180-idp.dtb: usb-vbus-regulator@1100: compatible:0: 'qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg' does not match '^[a-zA-Z0-9][a-zA-Z0-9,+\\-._/]+$'
> sc7180-idp.dtb: /soc@0/spmi@c440000/pmic@0/usb-vbus-regulator@1100: failed to match any schema with compatible: ['qcom,pm6150-vbus-reg,\n qcom,pm8150b-vbus-reg']
>
> Reported-by: Rob Herring <robh@kernel.org>
> Fixes: f81c2f01cad6 ("arm64: dts: qcom: pm6150: define USB-C related blocks")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/pm6150.dtsi | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/pm6150.dtsi b/arch/arm64/boot/dts/qcom/pm6150.dtsi
> index 11158c2bd524..b20a639cddf3 100644
> --- a/arch/arm64/boot/dts/qcom/pm6150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm6150.dtsi
> @@ -71,8 +71,8 @@ pm6150_vbus: usb-vbus-regulator@1100 {
> };
>
> pm6150_typec: typec@1500 {
> - compatible = "qcom,pm6150-typec,
> - qcom,pm8150b-typec";
> + compatible = "qcom,pm6150-typec",
> + "qcom,pm8150b-typec";
The warning in the commit description is related to the
'usb-vbus-regulator@1100' node, whereas this fixes the compatible of a different
one.
Did you want to fix both?
Regards,
Gabor
^ permalink raw reply
* Re: [PATCH 00/23] v2: imx258 improvement series
From: Luigi311 @ 2024-03-30 6:37 UTC (permalink / raw)
To: Dang Huynh, linux-media
Cc: dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel
In-Reply-To: <4599295.LvFx2qVVIh@melttower>
On 3/29/24 21:51, Dang Huynh wrote:
> On Wednesday, March 27, 2024 11:16:46 PM UTC git@luigi311.com wrote:
>> From: Luigi311 <git@luigi311.com>
>
> The Linux kernel does not allow anonymous (or pseudonymous) contributions. You
> should use your real first and last name.
>
> See section 1.5 in "A guide to the Kernel Development Process":
> https://www.kernel.org/doc/html/v6.8/process/1.Intro.html#licensing
>
>
Ok I've changed my sign off to my real first and last name for the
next revision
^ permalink raw reply
* Re: [PATCH RFC v2 0/4] wifi: ath10k: support board-specific firmware overrides
From: Dmitry Baryshkov @ 2024-03-30 4:47 UTC (permalink / raw)
To: Kalle Valo, Jeff Johnson, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: ath10k, linux-wireless, netdev, devicetree, linux-arm-msm,
Krzysztof Kozlowski
In-Reply-To: <20240306-wcn3990-firmware-path-v2-0-f89e98e71a57@linaro.org>
On Wed, 6 Mar 2024 at 10:16, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On WCN3990 platforms actual firmware, wlanmdsp.mbn, is sideloaded to the
> modem DSP via the TQFTPserv. These MBN files are signed by the device
> vendor, can only be used with the particular SoC or device.
>
> Unfortunately different firmware versions come with different features.
> For example firmware for SDM845 doesn't use single-chan-info-per-channel
> feature, while firmware for QRB2210 / QRB4210 requires that feature.
>
> Allow board DT files to override the subdir of the fw dir used to lookup
> the firmware-N.bin file decribing corresponding WiFi firmware.
> For example, adding firmware-name = "qrb4210" property will make the
> driver look for the firmware-N.bin first in ath10k/WCN3990/hw1.0/qrb4210
> directory and then fallback to the default ath10k/WCN3990/hw1.0 dir.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> Changes in v2:
> - Fixed the comment about the default board name being NULL (Kalle)
> - Expanded commit message to provide examples for firmware paths (Kalle)
> - Added a note regarding board-2.bin to the commit message (Kalle)
> - Link to v1: https://lore.kernel.org/r/20240130-wcn3990-firmware-path-v1-0-826b93202964@linaro.org
>
> ---
> Dmitry Baryshkov (4):
> dt-bindings: net: wireless: ath10k: describe firmware-name property
> wifi: ath10k: support board-specific firmware overrides
> arm64: dts: qcom: qrb2210-rb1: add firmware-name qualifier to WiFi node
> arm64: dts: qcom: qrb4210-rb1: add firmware-name qualifier to WiFi node
Kalle, Jeff, is there anything pending on me on this series?
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH] sh: j2: drop incorrect SPI controller max frequency property
From: Rob Landley @ 2024-03-30 4:44 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, devicetree, linux-sh, linux-kernel
Cc: Kousik Sanagavarapu
In-Reply-To: <20240322064221.25776-1-krzysztof.kozlowski@linaro.org>
On 3/22/24 01:42, Krzysztof Kozlowski wrote:
> The J2 SPI controller bindings never allowed spi-max-frequency property
> in the controller node. Neither old spi-bus.txt bindings, nor new DT
> schema allows it. Linux driver does not parse that property from
> controller node, thus drop it from DTS as incorrect hardware
> description. The SPI child device has already the same property with
> the same value, so functionality should not be affected.
>
> Cc: Kousik Sanagavarapu <five231003@gmail.com>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Tested-by: Rob Landley <rob@landley.net>
(By which I mean a 6.8 kernel with this patch applied booted to a shell prompt
on my J2 turtle board.)
Rob
^ permalink raw reply
* [PATCH v2 18/18] PCI: rockchip-ep: Handle PERST# signal in endpoint mode
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
Currently, the Rockchip PCIe endpoint controller driver does not handle
PERST# signal, which prevents detecting when link training should
actually be started or if the host reset the device. This however can
be supported using the controller ep_gpio, set as an input GPIO for
endpoint mode.
Modify the endpoint rockchip driver to get the ep_gpio and its
associated interrupt which is serviced using a threaded IRQ with the
function rockchip_pcie_ep_perst_irq_thread() as handler.
This handler function notifies a link down event corresponding to the RC
side asserting the PERST# signal using pci_epc_linkdown() when the gpio
is high. Once the gpio value goes down, corresponding to the RC
de-asserting the PERST# signal, link training is started. The polarity
of the gpio interrupt trigger is changed from high to low after the RC
asserted PERST#, and conversely changed from low to high after the RC
de-asserts PERST#.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/pci/controller/pcie-rockchip-ep.c | 118 +++++++++++++++++++++-
drivers/pci/controller/pcie-rockchip.c | 12 +--
2 files changed, 122 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 4006e7dee71a..e098c5fb6d59 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -18,6 +18,7 @@
#include <linux/sizes.h>
#include <linux/workqueue.h>
#include <linux/iopoll.h>
+#include <linux/gpio/consumer.h>
#include "pcie-rockchip.h"
@@ -50,6 +51,9 @@ struct rockchip_pcie_ep {
u64 irq_pci_addr;
u8 irq_pci_fn;
u8 irq_pending;
+ int perst_irq;
+ bool perst_asserted;
+ bool link_up;
struct delayed_work link_training;
};
@@ -464,13 +468,17 @@ static int rockchip_pcie_ep_start(struct pci_epc *epc)
rockchip_pcie_write(rockchip, cfg, PCIE_CORE_PHY_FUNC_CFG);
+ if (rockchip->ep_gpio)
+ enable_irq(ep->perst_irq);
+
/* Enable configuration and start link training */
rockchip_pcie_write(rockchip,
PCIE_CLIENT_LINK_TRAIN_ENABLE |
PCIE_CLIENT_CONF_ENABLE,
PCIE_CLIENT_CONFIG);
- schedule_delayed_work(&ep->link_training, 0);
+ if (!rockchip->ep_gpio)
+ schedule_delayed_work(&ep->link_training, 0);
return 0;
}
@@ -480,6 +488,11 @@ static void rockchip_pcie_ep_stop(struct pci_epc *epc)
struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
struct rockchip_pcie *rockchip = &ep->rockchip;
+ if (rockchip->ep_gpio) {
+ ep->perst_asserted = true;
+ disable_irq(ep->perst_irq);
+ }
+
cancel_delayed_work_sync(&ep->link_training);
/* Stop link training and disable configuration */
@@ -542,6 +555,13 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work)
if (!rockchip_pcie_ep_link_up(rockchip))
goto again;
+ /*
+ * If PERST was asserted while polling the link, do not notify
+ * the function.
+ */
+ if (ep->perst_asserted)
+ return;
+
val = rockchip_pcie_read(rockchip, PCIE_CLIENT_BASIC_STATUS0);
dev_info(dev,
"Link UP (Negociated speed: %sGT/s, width: x%lu)\n",
@@ -551,6 +571,7 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work)
/* Notify the function */
pci_epc_linkup(ep->epc);
+ ep->link_up = true;
return;
@@ -558,6 +579,94 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work)
schedule_delayed_work(&ep->link_training, msecs_to_jiffies(5));
}
+static void rockchip_pcie_ep_perst_assert(struct rockchip_pcie_ep *ep)
+{
+ struct rockchip_pcie *rockchip = &ep->rockchip;
+ struct device *dev = rockchip->dev;
+
+ dev_dbg(dev, "PERST asserted, link down\n");
+
+ if (ep->perst_asserted)
+ return;
+
+ ep->perst_asserted = true;
+
+ cancel_delayed_work_sync(&ep->link_training);
+
+ if (ep->link_up) {
+ pci_epc_linkdown(ep->epc);
+ ep->link_up = false;
+ }
+}
+
+static void rockchip_pcie_ep_perst_deassert(struct rockchip_pcie_ep *ep)
+{
+ struct rockchip_pcie *rockchip = &ep->rockchip;
+ struct device *dev = rockchip->dev;
+
+ dev_dbg(dev, "PERST de-asserted, starting link training\n");
+
+ if (!ep->perst_asserted)
+ return;
+
+ ep->perst_asserted = false;
+
+ /* Enable link re-training */
+ rockchip_pcie_ep_retrain_link(rockchip);
+
+ /* Start link training */
+ schedule_delayed_work(&ep->link_training, 0);
+}
+
+static irqreturn_t rockchip_pcie_ep_perst_irq_thread(int irq, void *data)
+{
+ struct pci_epc *epc = data;
+ struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
+ struct rockchip_pcie *rockchip = &ep->rockchip;
+ u32 perst = gpiod_get_value(rockchip->ep_gpio);
+
+ if (perst)
+ rockchip_pcie_ep_perst_assert(ep);
+ else
+ rockchip_pcie_ep_perst_deassert(ep);
+
+ irq_set_irq_type(ep->perst_irq,
+ (perst ? IRQF_TRIGGER_HIGH : IRQF_TRIGGER_LOW));
+
+ return IRQ_HANDLED;
+}
+
+static int rockchip_pcie_ep_setup_irq(struct pci_epc *epc)
+{
+ struct rockchip_pcie_ep *ep = epc_get_drvdata(epc);
+ struct rockchip_pcie *rockchip = &ep->rockchip;
+ struct device *dev = rockchip->dev;
+ int ret;
+
+ if (!rockchip->ep_gpio)
+ return 0;
+
+ /* PCIe reset interrupt */
+ ep->perst_irq = gpiod_to_irq(rockchip->ep_gpio);
+ if (ep->perst_irq < 0) {
+ dev_err(dev, "No corresponding IRQ for PERST GPIO\n");
+ return ep->perst_irq;
+ }
+
+ ep->perst_asserted = true;
+ irq_set_status_flags(ep->perst_irq, IRQ_NOAUTOEN);
+ ret = devm_request_threaded_irq(dev, ep->perst_irq, NULL,
+ rockchip_pcie_ep_perst_irq_thread,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ "pcie-ep-perst", epc);
+ if (ret) {
+ dev_err(dev, "Request PERST GPIO IRQ failed %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
static const struct pci_epc_features rockchip_pcie_epc_features = {
.linkup_notifier = true,
.msi_capable = true,
@@ -721,6 +830,7 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
rockchip->is_rc = false;
rockchip->dev = dev;
INIT_DELAYED_WORK(&ep->link_training, rockchip_pcie_ep_link_training);
+ ep->link_up = false;
epc = devm_pci_epc_create(dev, &rockchip_pcie_epc_ops);
if (IS_ERR(epc)) {
@@ -748,7 +858,13 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
/* Only enable function 0 by default */
rockchip_pcie_write(rockchip, BIT(0), PCIE_CORE_PHY_FUNC_CFG);
+ err = rockchip_pcie_ep_setup_irq(epc);
+ if (err < 0)
+ goto err_uninit_port;
+
return 0;
+err_uninit_port:
+ rockchip_pcie_deinit_phys(rockchip);
err_release_resources:
rockchip_pcie_ep_release_resources(ep);
err_disable_clocks:
diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
index dbec700ba9f9..3938c0b6b5a9 100644
--- a/drivers/pci/controller/pcie-rockchip.c
+++ b/drivers/pci/controller/pcie-rockchip.c
@@ -119,13 +119,11 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
return PTR_ERR(rockchip->aclk_rst);
}
- if (rockchip->is_rc) {
- rockchip->ep_gpio = devm_gpiod_get_optional(dev, "ep",
- GPIOD_OUT_HIGH);
- if (IS_ERR(rockchip->ep_gpio))
- return dev_err_probe(dev, PTR_ERR(rockchip->ep_gpio),
- "failed to get ep GPIO\n");
- }
+ rockchip->ep_gpio = devm_gpiod_get_optional(dev, "ep",
+ rockchip->is_rc ? GPIOD_OUT_HIGH : GPIOD_IN);
+ if (IS_ERR(rockchip->ep_gpio))
+ return dev_err_probe(dev, PTR_ERR(rockchip->ep_gpio),
+ "failed to get ep GPIO\n");
rockchip->aclk_pcie = devm_clk_get(dev, "aclk");
if (IS_ERR(rockchip->aclk_pcie)) {
--
2.44.0
^ permalink raw reply related
* [PATCH v2 17/18] dt-bindings: pci: rockchip,rk3399-pcie-ep: Add ep-gpios property
From: Damien Le Moal @ 2024-03-30 4:19 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree
Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-1-dlemoal@kernel.org>
From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Describe the `ep-gpios` property which is used to map the PERST# input
signal for endpoint mode.
Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
.../devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
index 6b62f6f58efe..9331d44d6963 100644
--- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
@@ -30,6 +30,9 @@ properties:
maximum: 32
default: 32
+ ep-gpios:
+ description: Input GPIO configured for the PERST# signal.
+
required:
- rockchip,max-outbound-regions
--
2.44.0
^ permalink raw reply related
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