Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH v3 09/23] dt-bindings: mfd: add samsung,s2dos05
From: Rob Herring (Arm) @ 2024-06-18 16:53 UTC (permalink / raw)
  To: Dzmitry Sankouski
  Cc: Dmitry Torokhov, Neil Armstrong, Pavel Machek, linux-input,
	Jessica Zhang, Mark Brown, Uwe Kleine-König,
	Michael Turquette, Konrad Dybcio, linux-arm-msm, Stephen Boyd,
	Lee Jones, linux-samsung-soc, Liam Girdwood, linux-clk,
	Maxime Ripard, linux-leds, Thomas Zimmermann, Chanwoo Choi,
	Maarten Lankhorst, Sebastian Reichel, Bjorn Andersson,
	phone-devel, Sam Ravnborg, Krzysztof Kozlowski, linux-kernel,
	David Airlie, Krzysztof Kozlowski, linux-pm, devicetree,
	linux-pwm, dri-devel, Conor Dooley, Daniel Vetter
In-Reply-To: <20240618-starqltechn_integration_upstream-v3-9-e3f6662017ac@gmail.com>


On Tue, 18 Jun 2024 16:59:43 +0300, Dzmitry Sankouski wrote:
> add samsung,s2dos05 core MFD module binding
> 
> Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> ---
>  .../devicetree/bindings/mfd/samsung,s2dos05.yaml   | 89 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 90 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/mfd/samsung,s2dos05.example.dtb: /example-0/i2c/pmic@60: failed to match any schema with compatible: ['samsung,s2dos05']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240618-starqltechn_integration_upstream-v3-9-e3f6662017ac@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply

* Re: [PATCH v5 2/2] dt-bindings: input: Goodix SPI HID Touchscreen
From: Conor Dooley @ 2024-06-18 17:43 UTC (permalink / raw)
  To: Charles Wang
  Cc: dmitry.torokhov, dan.carpenter, robh, dianders, krzk+dt, jikos,
	bentiss, hbarnor, linux-input, devicetree, linux-kernel
In-Reply-To: <20240618084455.1451461-3-charles.goodix@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2964 bytes --]

On Tue, Jun 18, 2024 at 04:44:54PM +0800, Charles Wang wrote:
> The Goodix GT7986U touch controller report touch data according to the
> HID protocol through the SPI bus. However, it is incompatible with
> Microsoft's HID-over-SPI protocol.
> 
> Signed-off-by: Charles Wang <charles.goodix@gmail.com>
> ---
>  .../bindings/input/goodix,gt7986u.yaml        | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> new file mode 100644
> index 000000000..25a26b4ce
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/goodix,gt7986u.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GOODIX GT7986U SPI HID Touchscreen
> +
> +maintainers:
> +  - Charles Wang <charles.goodix@gmail.com>
> +
> +description: Supports the Goodix GT7986U touchscreen.
> +  This touch controller reports data packaged according to the HID protocol,
> +  but is incompatible with Microsoft's HID-over-SPI protocol.
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - goodix,gt7986u
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: reset gpio the chip is connected to.

The description here isn't needed, it's obvious :)

> +  goodix,hid-report-addr:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |

This | is not needed, there's no formatting to preserve.

Those are nits:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> +      The register address for retrieving HID report data.
> +      This address is related to the device firmware and may
> +      change after a firmware update.
> +
> +  spi-max-frequency: true
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - reset-gpios
> +  - goodix,hid-report-addr
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      touchscreen@0 {
> +        compatible = "goodix,gt7986u";
> +        reg = <0>;
> +        interrupt-parent = <&gpio>;
> +        interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
> +        reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
> +        spi-max-frequency = <10000000>;
> +        goodix,hid-report-addr = <0x22c8c>;
> +      };
> +    };
> +
> +...
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v3 02/23] gcc-sdm845: Add rates to the GP clocks
From: Dmitry Baryshkov @ 2024-06-18 17:50 UTC (permalink / raw)
  To: Dzmitry Sankouski
  Cc: Sebastian Reichel, Bjorn Andersson, Michael Turquette,
	Stephen Boyd, Neil Armstrong, Jessica Zhang, Sam Ravnborg,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Daniel Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones, Dmitry Torokhov, Pavel Machek, Liam Girdwood,
	Mark Brown, Uwe Kleine-König, Krzysztof Kozlowski,
	Konrad Dybcio, Chanwoo Choi, phone-devel, linux-pm, linux-kernel,
	linux-arm-msm, linux-clk, dri-devel, devicetree, linux-input,
	linux-leds, linux-pwm, linux-samsung-soc
In-Reply-To: <20240618-starqltechn_integration_upstream-v3-2-e3f6662017ac@gmail.com>

On Tue, Jun 18, 2024 at 04:59:36PM GMT, Dzmitry Sankouski wrote:
> sdm845 has "General Purpose" clocks that can be muxed to
> SoC pins.
> 
> Those clocks may be used as e.g. PWM sources for external peripherals.
> Add more frequencies to the table for those clocks so it's possible
> for arbitrary peripherals to make use of them.
> 
> See also: bf8bb8eaccf(clk: qcom: gcc-msm8916: Add rates to the GP clocks)

Each time I look at the table attached to the GP CLK, I feel that it's
plain wrong. In the end the GPCLK can in theory have arbitrary value
depending on the usecase.

Bjorn, Konrad, maybe we should add special clk_ops for GP CLK which
allow more flexibility than a default clk_rcg2_ops?

> 
> Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> ---
>  drivers/clk/qcom/gcc-sdm845.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
> index ea4c3bf4fb9b..0efd3364e8f5 100644
> --- a/drivers/clk/qcom/gcc-sdm845.c
> +++ b/drivers/clk/qcom/gcc-sdm845.c
> @@ -283,7 +283,21 @@ static struct clk_rcg2 gcc_sdm670_cpuss_rbcpr_clk_src = {
>  	},
>  };
>  
> +/*
> + * This is a frequency table for "General Purpose" clocks.
> + * These clocks can be muxed to the SoC pins and may be used by
> + * external devices. They're often used as PWM source.
> + *
> + * See comment in gcc-mam8916.c at ftbl_gcc_gp1_3_clk.
> + */
>  static const struct freq_tbl ftbl_gcc_gp1_clk_src[] = {
> +	F(10000,   P_BI_TCXO,    16,  1, 120),
> +	F(20000,   P_BI_TCXO,    16,  1, 60),
> +	F(100000,  P_BI_TCXO,    16,  1,  12),
> +	F(500000,  P_GPLL0_OUT_EVEN, 12, 1, 100),
> +	F(1000000, P_GPLL0_OUT_EVEN, 12, 1, 50),
> +	F(2500000, P_GPLL0_OUT_EVEN, 12, 1, 10),
> +	F(5000000, P_GPLL0_OUT_EVEN, 12, 1, 5),
>  	F(19200000, P_BI_TCXO, 1, 0, 0),
>  	F(25000000, P_GPLL0_OUT_EVEN, 12, 0, 0),
>  	F(50000000, P_GPLL0_OUT_EVEN, 6, 0, 0),
> 
> -- 
> 2.39.2
> 

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH v10 00/38] ep93xx device tree conversion
From: Jakub Kicinski @ 2024-06-18 18:08 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Nikita Shubin via B4 Relay, Arnd Bergmann, Hartley Sweeten,
	Alexander Sverdlin, Russell King, Lukasz Majewski, Linus Walleij,
	Bartosz Golaszewski, Andy Shevchenko, Michael Turquette,
	Stephen Boyd, Sebastian Reichel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Vinod Koul, Wim Van Sebroeck, Guenter Roeck,
	Thierry Reding, Uwe Kleine-König, Mark Brown,
	David S. Miller, Eric Dumazet, Paolo Abeni, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Damien Le Moal,
	Sergey Shtylyov, Dmitry Torokhov, Liam Girdwood, Jaroslav Kysela,
	Takashi Iwai, Ralf Baechle, Wu, Aaron, Lee Jones, Olof Johansson,
	Niklas Cassel, linux-arm-kernel, linux-kernel, linux-gpio,
	linux-clk, linux-pm, devicetree, dmaengine, linux-watchdog,
	linux-pwm, linux-spi, netdev, linux-mtd, linux-ide, linux-input,
	linux-sound, Bartosz Golaszewski, Krzysztof Kozlowski,
	Andy Shevchenko, Andy Shevchenko, Andrew Lunn
In-Reply-To: <eb3e6c0b883f408fed68e725a23b54854701ce9e.camel@maquefel.me>

On Tue, 18 Jun 2024 19:33:49 +0300 Nikita Shubin wrote:
> On Tue, 2024-06-18 at 07:33 -0700, Jakub Kicinski wrote:
> > On Mon, 17 Jun 2024 12:36:34 +0300 Nikita Shubin via B4 Relay wrote:  
> > > The goal is to recieve ACKs for all patches in series to merge it
> > > via Arnd branch.  
> > 
> > Why? The usual process is for every subsystem to accept the relevant
> > patches, and then they converge during the merge window.  
> 
> It was decided from the very beginning of these series, mostly because
> it's a full conversion of platform code to DT and it seemed not
> convenient to maintain compatibility with both platform and DT.
> 
> Generally i think it's too late to ask such a question, when just a few
> patches left.

Put the relevant information in the cover letter. Justification why you
can't follow normal merging rules is very relevant.

^ permalink raw reply

* Re: [PATCH v3 11/23] drm/panel: Add support for S6E3HA8 panel driver
From: Dmitry Baryshkov @ 2024-06-18 18:39 UTC (permalink / raw)
  To: Dzmitry Sankouski
  Cc: Sebastian Reichel, Bjorn Andersson, Michael Turquette,
	Stephen Boyd, Neil Armstrong, Jessica Zhang, Sam Ravnborg,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Daniel Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones, Dmitry Torokhov, Pavel Machek, Liam Girdwood,
	Mark Brown, Uwe Kleine-König, Krzysztof Kozlowski,
	Konrad Dybcio, Chanwoo Choi, phone-devel, linux-pm, linux-kernel,
	linux-arm-msm, linux-clk, dri-devel, devicetree, linux-input,
	linux-leds, linux-pwm, linux-samsung-soc
In-Reply-To: <20240618-starqltechn_integration_upstream-v3-11-e3f6662017ac@gmail.com>

On Tue, Jun 18, 2024 at 04:59:45PM GMT, Dzmitry Sankouski wrote:
> Add support for MIPI-DSI based S6E3HA8 AMOLED panel
> driver. This panel has 1440x2960 resolution, 5.8-inch physical
> size, and can be found in starqltechn device.
> Brightness regulation is not yet supported.
> 
> Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> ---
>  MAINTAINERS                                   |   1 +
>  drivers/gpu/drm/panel/Kconfig                 |   7 +
>  drivers/gpu/drm/panel/Makefile                |   1 +
>  drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c | 426 ++++++++++++++++++++++++++
>  4 files changed, 435 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 92a5d0a56353..fae3b8ea9ce4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7154,6 +7154,7 @@ DRM DRIVER FOR SAMSUNG S6E3HA8 PANELS
>  M:	Dzmitry Sankouski <dsankouski@gmail.com>
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/display/panel/samsung,s6e3ha8.yaml
> +F:	drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
>  
>  DRM DRIVER FOR SITRONIX ST7586 PANELS
>  M:	David Lechner <david@lechnology.com>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 2ae0eb0638f3..903fc8c155c0 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -651,6 +651,13 @@ config DRM_PANEL_SAMSUNG_S6E3HA2
>  	depends on BACKLIGHT_CLASS_DEVICE
>  	select VIDEOMODE_HELPERS
>  
> +config DRM_PANEL_SAMSUNG_S6E3HA8
> +	tristate "Samsung S6E3HA8 DSI video mode panel"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	select VIDEOMODE_HELPERS
> +
>  config DRM_PANEL_SAMSUNG_S6E63J0X03
>  	tristate "Samsung S6E63J0X03 DSI command mode panel"
>  	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index f0203f6e02f4..71774cc5820a 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D27A1) += panel-samsung-s6d27a1.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D7AA0) += panel-samsung-s6d7aa0.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3FA7) += panel-samsung-s6e3fa7.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
> +obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA8) += panel-samsung-s6e3ha8.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63J0X03) += panel-samsung-s6e63j0x03.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0) += panel-samsung-s6e63m0.o
>  obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E63M0_SPI) += panel-samsung-s6e63m0-spi.o
> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c b/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
> new file mode 100644
> index 000000000000..49d629643171
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
> @@ -0,0 +1,426 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Generated with linux-mdss-dsi-panel-driver-generator from vendor device tree:
> + *  Copyright (c) 2013, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024 Dzmitry Sankouski <dsankouski@gmail.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <drm/display/drm_dsc.h>
> +#include <drm/display/drm_dsc_helper.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +
> +struct s6e3ha8 {
> +	struct drm_panel panel;
> +	struct mipi_dsi_device *dsi;
> +	struct drm_dsc_config dsc;
> +	struct gpio_desc *reset_gpio;
> +	struct regulator_bulk_data supplies[3];
> +};
> +
> +static inline
> +struct s6e3ha8 *to_s6e3ha8_amb577px01_wqhd(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct s6e3ha8, panel);
> +}
> +
> +#define s6e3ha8_call_write_func(ret, func) do {	\
> +	ret = (func);				\
> +	if (ret < 0)				\
> +		return ret;			\
> +} while (0)

Please rework the driver to use mipi_dsi_*_multi() family of functions.
Using the mipi_dsi_multi_context should make this wrapper obsolete too.

> +
> +static int s6e3ha8_test_key_on_lvl1(struct mipi_dsi_device *dsi)
> +{
> +	static const u8 d[] = { 0x9f, 0xa5, 0xa5 };
> +
> +	return mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d));
> +	return 0;

Ugh. So which return is it?

> +}
> +

[...]
> +static int s6e3ha8_power_on(struct s6e3ha8 *ctx)
> +{
> +	int ret;
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);

Inline this function, it's just regulator_bulk_enable() in the end.

> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int s6e3ha8_power_off(struct s6e3ha8 *ctx)
> +{
> +	return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +}
> +
> +static void s6e3ha8_amb577px01_wqhd_reset(struct s6e3ha8 *ctx)
> +{
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +	usleep_range(5000, 6000);
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +	usleep_range(5000, 6000);
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +	usleep_range(5000, 6000);
> +}
> +
> +static int s6e3ha8_amb577px01_wqhd_on(struct s6e3ha8 *ctx)
> +{
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	struct device *dev = &dsi->dev;
> +	int ret;
> +
> +	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +	s6e3ha8_test_key_on_lvl1(dsi);
> +	s6e3ha8_test_key_on_lvl2(dsi);
> +
> +	ret = mipi_dsi_compression_mode(dsi, true);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to set compression mode: %d\n", ret);
> +		return ret;
> +	}

Interesting, compression mode is being set before the PPS programming?

> +
> +	s6e3ha8_test_key_off_lvl2(dsi);
> +
> +	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
> +		return ret;
> +	}
> +	usleep_range(5000, 6000);
> +
> +	s6e3ha8_test_key_on_lvl2(dsi);
> +	mipi_dsi_generic_write_seq(dsi, 0xf2, 0x13);
> +	s6e3ha8_test_key_off_lvl2(dsi);
> +
> +	usleep_range(10000, 11000);
> +
> +	s6e3ha8_test_key_on_lvl2(dsi);
> +	mipi_dsi_generic_write_seq(dsi, 0xf2, 0x13);
> +	s6e3ha8_test_key_off_lvl2(dsi);
> +
> +	/* OMOK setting 1 (Initial setting) - Scaler Latch Setting Guide */
> +	s6e3ha8_test_key_on_lvl2(dsi);
> +	mipi_dsi_generic_write_seq(dsi, 0xb0, 0x07);
> +	/* latch setting 1 : Scaler on/off & address setting & PPS setting -> Image update latch */
> +	mipi_dsi_generic_write_seq(dsi, 0xf2, 0x3c, 0x10);
> +	mipi_dsi_generic_write_seq(dsi, 0xb0, 0x0b);
> +	/* latch setting 2 : Ratio change mode -> Image update latch */
> +	mipi_dsi_generic_write_seq(dsi, 0xf2, 0x30);
> +	/* OMOK setting 2 - Seamless setting guide : WQHD */
> +	mipi_dsi_generic_write_seq(dsi, 0x2a, 0x00, 0x00, 0x05, 0x9f); /* CASET */
> +	mipi_dsi_generic_write_seq(dsi, 0x2b, 0x00, 0x00, 0x0b, 0x8f); /* PASET */
> +	mipi_dsi_generic_write_seq(dsi, 0xba, 0x01); /* scaler setup : scaler off */
> +	s6e3ha8_test_key_off_lvl2(dsi);
> +	mipi_dsi_generic_write_seq(dsi, 0x35, 0x00); /* TE Vsync ON */
> +	s6e3ha8_test_key_on_lvl2(dsi);
> +	mipi_dsi_generic_write_seq(dsi, 0xed, 0x4c); /* ERR_FG */
> +	s6e3ha8_test_key_off_lvl2(dsi);
> +	s6e3ha8_test_key_on_lvl3(dsi);
> +	/* FFC Setting 897.6Mbps */
> +	mipi_dsi_generic_write_seq(dsi, 0xc5, 0x0d, 0x10, 0xb4, 0x3e, 0x01);
> +	s6e3ha8_test_key_off_lvl3(dsi);
> +	s6e3ha8_test_key_on_lvl2(dsi);
> +	mipi_dsi_generic_write_seq(dsi, 0xb9,
> +				   0x00, 0xb0, 0x81, 0x09, 0x00, 0x00, 0x00,
> +				   0x11, 0x03); /* TSP HSYNC Setting */
> +	s6e3ha8_test_key_off_lvl2(dsi);
> +	s6e3ha8_test_key_on_lvl2(dsi);
> +	mipi_dsi_generic_write_seq(dsi, 0xb0, 0x03);
> +	mipi_dsi_generic_write_seq(dsi, 0xf6, 0x43);
> +	s6e3ha8_test_key_off_lvl2(dsi);
> +	s6e3ha8_test_key_on_lvl2(dsi);
> +	/* Brightness condition set */
> +	mipi_dsi_generic_write_seq(dsi, 0xca,
> +				   0x07, 0x00, 0x00, 0x00, 0x80, 0x80, 0x80,
> +				   0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80,
> +				   0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80,
> +				   0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80,
> +				   0x80, 0x80, 0x80, 0x00, 0x00, 0x00);
> +	mipi_dsi_generic_write_seq(dsi, 0xb1, 0x00, 0x0c); /* AID Set : 0% */
> +	mipi_dsi_generic_write_seq(dsi, 0xb5,
> +				   0x19, 0xdc, 0x16, 0x01, 0x34, 0x67, 0x9a,
> +				   0xcd, 0x01, 0x22, 0x33, 0x44, 0x00, 0x00,
> +				   0x05, 0x55, 0xcc, 0x0c, 0x01, 0x11, 0x11,
> +				   0x10); /* MPS/ELVSS Setting */
> +	mipi_dsi_generic_write_seq(dsi, 0xf4, 0xeb, 0x28); /* VINT */
> +	mipi_dsi_generic_write_seq(dsi, 0xf7, 0x03); /* Gamma, LTPS(AID) update */
> +	s6e3ha8_test_key_off_lvl2(dsi);
> +	s6e3ha8_test_key_off_lvl1(dsi);
> +
> +	return 0;
> +}
> +
> +static int s6e3ha8_enable(struct drm_panel *panel)
> +{
> +	struct s6e3ha8 *ctx = to_s6e3ha8_amb577px01_wqhd(panel);
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	int ret;
> +
> +	s6e3ha8_call_write_func(ret, s6e3ha8_test_key_on_lvl1(dsi));
> +	s6e3ha8_call_write_func(ret, mipi_dsi_dcs_set_display_on(dsi));
> +	s6e3ha8_call_write_func(ret, s6e3ha8_test_key_off_lvl1(dsi));
> +
> +	return 0;
> +}
> +
> +static int s6e3ha8_disable(struct drm_panel *panel)
> +{
> +	struct s6e3ha8 *ctx = to_s6e3ha8_amb577px01_wqhd(panel);
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	int ret;
> +
> +	s6e3ha8_call_write_func(ret, s6e3ha8_test_key_on_lvl1(dsi));
> +	s6e3ha8_call_write_func(ret, mipi_dsi_dcs_set_display_off(dsi));
> +	s6e3ha8_call_write_func(ret, s6e3ha8_test_key_off_lvl1(dsi));
> +	msleep(20);
> +
> +	s6e3ha8_call_write_func(ret, s6e3ha8_test_key_on_lvl2(dsi));
> +	s6e3ha8_call_write_func(ret, s6e3ha8_afc_off(dsi));
> +	s6e3ha8_call_write_func(ret, s6e3ha8_test_key_off_lvl2(dsi));
> +
> +	msleep(160);
> +
> +	return 0;
> +}
> +
> +static int s6e3ha8_amb577px01_wqhd_prepare(struct drm_panel *panel)
> +{
> +	struct s6e3ha8 *ctx = to_s6e3ha8_amb577px01_wqhd(panel);
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	struct device *dev = &dsi->dev;
> +	struct drm_dsc_picture_parameter_set pps;
> +	int ret;
> +
> +	s6e3ha8_power_on(ctx);
> +	msleep(120);
> +	s6e3ha8_amb577px01_wqhd_reset(ctx);
> +	ret = s6e3ha8_amb577px01_wqhd_on(ctx);
> +
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to initialize panel: %d\n", ret);
> +		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +		goto err;
> +	}
> +
> +	drm_dsc_pps_payload_pack(&pps, &ctx->dsc);
> +
> +	s6e3ha8_test_key_on_lvl1(dsi);
> +	ret = mipi_dsi_picture_parameter_set(ctx->dsi, &pps);
> +	if (ret < 0) {
> +		dev_err(panel->dev, "failed to transmit PPS: %d\n", ret);
> +		return ret;
> +	}
> +	s6e3ha8_test_key_off_lvl1(dsi);
> +
> +	ret = mipi_dsi_compression_mode(ctx->dsi, true);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to enable compression mode: %d\n", ret);
> +		return ret;
> +	}

Again?

> +
> +
> +	msleep(28);
> +
> +	return 0;
> +err:
> +	s6e3ha8_power_off(ctx);
> +	return ret;
> +}
> +
> +static int s6e3ha8_amb577px01_wqhd_unprepare(struct drm_panel *panel)
> +{
> +	struct s6e3ha8 *ctx = to_s6e3ha8_amb577px01_wqhd(panel);
> +
> +	return s6e3ha8_power_off(ctx);
> +}
> +
> +static const struct drm_display_mode s6e3ha8_amb577px01_wqhd_mode = {
> +	.clock = (1440 + 116 + 44 + 120) * (2960 + 120 + 80 + 124) * 60 / 1000,
> +	.hdisplay = 1440,
> +	.hsync_start = 1440 + 116,
> +	.hsync_end = 1440 + 116 + 44,
> +	.htotal = 1440 + 116 + 44 + 120,
> +	.vdisplay = 2960,
> +	.vsync_start = 2960 + 120,
> +	.vsync_end = 2960 + 120 + 80,
> +	.vtotal = 2960 + 120 + 80 + 124,
> +	.width_mm = 64,
> +	.height_mm = 132,
> +};
> +
> +static int s6e3ha8_amb577px01_wqhd_get_modes(struct drm_panel *panel,
> +					     struct drm_connector *connector)
> +{
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_duplicate(connector->dev, &s6e3ha8_amb577px01_wqhd_mode);
> +	if (!mode)
> +		return -ENOMEM;
> +
> +	drm_mode_set_name(mode);
> +
> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +	connector->display_info.width_mm = mode->width_mm;
> +	connector->display_info.height_mm = mode->height_mm;
> +	drm_mode_probed_add(connector, mode);

drm_connector_helper_get_modes_fixed()

> +
> +	return 1;
> +}
> +

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH v3 02/23] gcc-sdm845: Add rates to the GP clocks
From: Konrad Dybcio @ 2024-06-18 18:50 UTC (permalink / raw)
  To: Dmitry Baryshkov, Dzmitry Sankouski
  Cc: Sebastian Reichel, Bjorn Andersson, Michael Turquette,
	Stephen Boyd, Neil Armstrong, Jessica Zhang, Sam Ravnborg,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Daniel Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones, Dmitry Torokhov, Pavel Machek, Liam Girdwood,
	Mark Brown, Uwe Kleine-König, Krzysztof Kozlowski,
	Chanwoo Choi, phone-devel, linux-pm, linux-kernel, linux-arm-msm,
	linux-clk, dri-devel, devicetree, linux-input, linux-leds,
	linux-pwm, linux-samsung-soc
In-Reply-To: <wnf3mfgdm4p4f5wrxdtlx4wccnizdvohc7iiyu5t22eeb67r57@xun3r73hksrg>



On 6/18/24 19:50, Dmitry Baryshkov wrote:
> On Tue, Jun 18, 2024 at 04:59:36PM GMT, Dzmitry Sankouski wrote:
>> sdm845 has "General Purpose" clocks that can be muxed to
>> SoC pins.
>>
>> Those clocks may be used as e.g. PWM sources for external peripherals.
>> Add more frequencies to the table for those clocks so it's possible
>> for arbitrary peripherals to make use of them.
>>
>> See also: bf8bb8eaccf(clk: qcom: gcc-msm8916: Add rates to the GP clocks)
> 
> Each time I look at the table attached to the GP CLK, I feel that it's
> plain wrong. In the end the GPCLK can in theory have arbitrary value
> depending on the usecase.
> 
> Bjorn, Konrad, maybe we should add special clk_ops for GP CLK which
> allow more flexibility than a default clk_rcg2_ops?

If we can somehow get max m/n/d values for all possible parents, sure

Konrad

^ permalink raw reply

* Re: [PATCH v3 02/23] gcc-sdm845: Add rates to the GP clocks
From: Dmitry Baryshkov @ 2024-06-18 18:55 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Dzmitry Sankouski, Sebastian Reichel, Bjorn Andersson,
	Michael Turquette, Stephen Boyd, Neil Armstrong, Jessica Zhang,
	Sam Ravnborg, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Dmitry Torokhov, Pavel Machek,
	Liam Girdwood, Mark Brown, Uwe Kleine-König,
	Krzysztof Kozlowski, Chanwoo Choi, phone-devel, linux-pm,
	linux-kernel, linux-arm-msm, linux-clk, dri-devel, devicetree,
	linux-input, linux-leds, linux-pwm, linux-samsung-soc
In-Reply-To: <ad04e203-4244-4cd3-9c9a-fae002962990@linaro.org>

On Tue, Jun 18, 2024 at 08:50:52PM GMT, Konrad Dybcio wrote:
> 
> 
> On 6/18/24 19:50, Dmitry Baryshkov wrote:
> > On Tue, Jun 18, 2024 at 04:59:36PM GMT, Dzmitry Sankouski wrote:
> > > sdm845 has "General Purpose" clocks that can be muxed to
> > > SoC pins.
> > > 
> > > Those clocks may be used as e.g. PWM sources for external peripherals.
> > > Add more frequencies to the table for those clocks so it's possible
> > > for arbitrary peripherals to make use of them.
> > > 
> > > See also: bf8bb8eaccf(clk: qcom: gcc-msm8916: Add rates to the GP clocks)
> > 
> > Each time I look at the table attached to the GP CLK, I feel that it's
> > plain wrong. In the end the GPCLK can in theory have arbitrary value
> > depending on the usecase.
> > 
> > Bjorn, Konrad, maybe we should add special clk_ops for GP CLK which
> > allow more flexibility than a default clk_rcg2_ops?
> 
> If we can somehow get max m/n/d values for all possible parents, sure

Calculate them at runtime?


-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH v3 02/23] gcc-sdm845: Add rates to the GP clocks
From: Konrad Dybcio @ 2024-06-18 19:11 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Dzmitry Sankouski, Sebastian Reichel, Bjorn Andersson,
	Michael Turquette, Stephen Boyd, Neil Armstrong, Jessica Zhang,
	Sam Ravnborg, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Dmitry Torokhov, Pavel Machek,
	Liam Girdwood, Mark Brown, Uwe Kleine-König,
	Krzysztof Kozlowski, Chanwoo Choi, phone-devel, linux-pm,
	linux-kernel, linux-arm-msm, linux-clk, dri-devel, devicetree,
	linux-input, linux-leds, linux-pwm, linux-samsung-soc
In-Reply-To: <lwrz4rvn6ogseea5v6j7plc3yi3xnzo76dvrsl3muat3iswlkb@zmwa3xo3xgw4>



On 6/18/24 20:55, Dmitry Baryshkov wrote:
> On Tue, Jun 18, 2024 at 08:50:52PM GMT, Konrad Dybcio wrote:
>>
>>
>> On 6/18/24 19:50, Dmitry Baryshkov wrote:
>>> On Tue, Jun 18, 2024 at 04:59:36PM GMT, Dzmitry Sankouski wrote:
>>>> sdm845 has "General Purpose" clocks that can be muxed to
>>>> SoC pins.
>>>>
>>>> Those clocks may be used as e.g. PWM sources for external peripherals.
>>>> Add more frequencies to the table for those clocks so it's possible
>>>> for arbitrary peripherals to make use of them.
>>>>
>>>> See also: bf8bb8eaccf(clk: qcom: gcc-msm8916: Add rates to the GP clocks)
>>>
>>> Each time I look at the table attached to the GP CLK, I feel that it's
>>> plain wrong. In the end the GPCLK can in theory have arbitrary value
>>> depending on the usecase.
>>>
>>> Bjorn, Konrad, maybe we should add special clk_ops for GP CLK which
>>> allow more flexibility than a default clk_rcg2_ops?
>>
>> If we can somehow get max m/n/d values for all possible parents, sure
> 
> Calculate them at runtime?

We'd be calculating the mnd values for a frequency that's either equal or
reasonably close to the one requested. My worry is that we somehow need
to get the maximum values they can take (unless they're well-known)

Konrad

^ permalink raw reply

* Re: [PATCH v5 2/2] dt-bindings: input: Goodix SPI HID Touchscreen
From: Charles Wang @ 2024-06-19  2:16 UTC (permalink / raw)
  To: Conor Dooley
  Cc: dmitry.torokhov, dan.carpenter, robh, dianders, krzk+dt, jikos,
	bentiss, hbarnor, linux-input, devicetree, linux-kernel
In-Reply-To: <20240618-affluent-unroasted-e6d5d34d1ea2@spud>

Hi Conor,

On Tue, Jun 18, 2024 at 06:43:18PM +0100, Conor Dooley wrote:
> On Tue, Jun 18, 2024 at 04:44:54PM +0800, Charles Wang wrote:
> > The Goodix GT7986U touch controller report touch data according to the
> > HID protocol through the SPI bus. However, it is incompatible with
> > Microsoft's HID-over-SPI protocol.
> > 
> > Signed-off-by: Charles Wang <charles.goodix@gmail.com>
> > ---
> >  .../bindings/input/goodix,gt7986u.yaml        | 72 +++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> > new file mode 100644
> > index 000000000..25a26b4ce
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> > @@ -0,0 +1,72 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/input/goodix,gt7986u.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: GOODIX GT7986U SPI HID Touchscreen
> > +
> > +maintainers:
> > +  - Charles Wang <charles.goodix@gmail.com>
> > +
> > +description: Supports the Goodix GT7986U touchscreen.
> > +  This touch controller reports data packaged according to the HID protocol,
> > +  but is incompatible with Microsoft's HID-over-SPI protocol.
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - goodix,gt7986u
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> > +    description: reset gpio the chip is connected to.
> 
> The description here isn't needed, it's obvious :)
>

Ack.

> > +  goodix,hid-report-addr:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> 
> This | is not needed, there's no formatting to preserve.
> 
> Those are nits:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> 

Thank you. If there is a next version, I will fix these.

Charles,

> 
> > +      The register address for retrieving HID report data.
> > +      This address is related to the device firmware and may
> > +      change after a firmware update.
> > +
> > +  spi-max-frequency: true
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - reset-gpios
> > +  - goodix,hid-report-addr
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    spi {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +
> > +      touchscreen@0 {
> > +        compatible = "goodix,gt7986u";
> > +        reg = <0>;
> > +        interrupt-parent = <&gpio>;
> > +        interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
> > +        reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
> > +        spi-max-frequency = <10000000>;
> > +        goodix,hid-report-addr = <0x22c8c>;
> > +      };
> > +    };
> > +
> > +...
> > -- 
> > 2.43.0
> > 



^ permalink raw reply

* Re: [PATCH v3 02/23] gcc-sdm845: Add rates to the GP clocks
From: Dmitry Baryshkov @ 2024-06-19  6:31 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Dzmitry Sankouski, Sebastian Reichel, Bjorn Andersson,
	Michael Turquette, Stephen Boyd, Neil Armstrong, Jessica Zhang,
	Sam Ravnborg, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Dmitry Torokhov, Pavel Machek,
	Liam Girdwood, Mark Brown, Uwe Kleine-König,
	Krzysztof Kozlowski, Chanwoo Choi, phone-devel, linux-pm,
	linux-kernel, linux-arm-msm, linux-clk, dri-devel, devicetree,
	linux-input, linux-leds, linux-pwm, linux-samsung-soc
In-Reply-To: <85e03d10-59a2-4f15-bb85-7b2c0354a5d1@linaro.org>

On Tue, Jun 18, 2024 at 09:11:58PM GMT, Konrad Dybcio wrote:
> 
> 
> On 6/18/24 20:55, Dmitry Baryshkov wrote:
> > On Tue, Jun 18, 2024 at 08:50:52PM GMT, Konrad Dybcio wrote:
> > > 
> > > 
> > > On 6/18/24 19:50, Dmitry Baryshkov wrote:
> > > > On Tue, Jun 18, 2024 at 04:59:36PM GMT, Dzmitry Sankouski wrote:
> > > > > sdm845 has "General Purpose" clocks that can be muxed to
> > > > > SoC pins.
> > > > > 
> > > > > Those clocks may be used as e.g. PWM sources for external peripherals.
> > > > > Add more frequencies to the table for those clocks so it's possible
> > > > > for arbitrary peripherals to make use of them.
> > > > > 
> > > > > See also: bf8bb8eaccf(clk: qcom: gcc-msm8916: Add rates to the GP clocks)
> > > > 
> > > > Each time I look at the table attached to the GP CLK, I feel that it's
> > > > plain wrong. In the end the GPCLK can in theory have arbitrary value
> > > > depending on the usecase.
> > > > 
> > > > Bjorn, Konrad, maybe we should add special clk_ops for GP CLK which
> > > > allow more flexibility than a default clk_rcg2_ops?
> > > 
> > > If we can somehow get max m/n/d values for all possible parents, sure
> > 
> > Calculate them at runtime?
> 
> We'd be calculating the mnd values for a frequency that's either equal or
> reasonably close to the one requested. My worry is that we somehow need
> to get the maximum values they can take (unless they're well-known)

One of the options might be to force devices to use
assigned-clock-parent to set GP CLK sorource and pwm-clk as an actual
device using the clock.

-- 
With best wishes
Dmitry

^ permalink raw reply

* [PATCH 1/1] Input: ads7846 - Use spi_device_id table
From: Alexander Stein @ 2024-06-19 12:27 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Alexander Stein, linux-input, linux-kernel

As the driver supports more devices over time the single MODULE_ALIAS
is complete and raises several warnings:
SPI driver ads7846 has no spi_device_id for ti,tsc2046
SPI driver ads7846 has no spi_device_id for ti,ads7843
SPI driver ads7846 has no spi_device_id for ti,ads7845
SPI driver ads7846 has no spi_device_id for ti,ads7873

Fix this by adding a spi_device_id table and removing the manual
MODULE_ALIAS.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 drivers/input/touchscreen/ads7846.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index d2bbb436a77df..4d13db13b9e57 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -1111,6 +1111,16 @@ static const struct of_device_id ads7846_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, ads7846_dt_ids);
 
+static const struct spi_device_id ads7846_spi_ids[] = {
+	{ "tsc2046", 7846 },
+	{ "ads7843", 7843 },
+	{ "ads7845", 7845 },
+	{ "ads7846", 7846 },
+	{ "ads7873", 7873 },
+	{ },
+};
+MODULE_DEVICE_TABLE(spi, ads7846_spi_ids);
+
 static const struct ads7846_platform_data *ads7846_get_props(struct device *dev)
 {
 	struct ads7846_platform_data *pdata;
@@ -1386,10 +1396,10 @@ static struct spi_driver ads7846_driver = {
 	},
 	.probe		= ads7846_probe,
 	.remove		= ads7846_remove,
+	.id_table	= ads7846_spi_ids,
 };
 
 module_spi_driver(ads7846_driver);
 
 MODULE_DESCRIPTION("ADS7846 TouchScreen Driver");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS("spi:ads7846");
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v3 11/23] drm/panel: Add support for S6E3HA8 panel driver
From: Dzmitry Sankouski @ 2024-06-19 13:27 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Sebastian Reichel, Bjorn Andersson, Michael Turquette,
	Stephen Boyd, Neil Armstrong, Jessica Zhang, Sam Ravnborg,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Daniel Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones, Dmitry Torokhov, Pavel Machek, Liam Girdwood,
	Mark Brown, Uwe Kleine-König, Krzysztof Kozlowski,
	Konrad Dybcio, Chanwoo Choi, phone-devel, linux-pm, linux-kernel,
	linux-arm-msm, linux-clk, dri-devel, devicetree, linux-input,
	linux-leds, linux-pwm, linux-samsung-soc
In-Reply-To: <pkmxbxoc4sno6mbjsftz6hp5lxefc6yhwxjlhiy2pd4wbkzpvl@as43z4t64mm6>

вт, 18 июн. 2024 г. в 21:39, Dmitry Baryshkov <dmitry.baryshkov@linaro.org>:
>
> > +     ret = mipi_dsi_compression_mode(dsi, true);
> > +     if (ret < 0) {
> > +             dev_err(dev, "Failed to set compression mode: %d\n", ret);
> > +             return ret;
> > +     }
>
> Interesting, compression mode is being set before the PPS programming?
>
Yes, as per vendor kernel:
https://github.com/klabit87/twrp_android_samsung_kernel_sdm845/blob/e8bb63039008e1704a2f1bde68d39ded9c16ea88/drivers/gpu/drm/msm/samsung/S6E3HA8_AMB577PX01/dsi_panel_S6E3HA8_AMB577PX01_wqhd_octa_cmd.dtsi#L5508

^ permalink raw reply

* Re: [PATCH AUTOSEL 6.9 31/35] HID: bpf: add in-tree HID-BPF fix for the HP Elite Presenter Mouse
From: Sasha Levin @ 2024-06-19 14:29 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: linux-kernel, stable, Peter Hutterer, jikos, linux-input, bpf
In-Reply-To: <k4j2pjxg23i6tggjc7beodock2q7pbjbgzombidzlowb7rpr37@ziaqvisevijj>

On Mon, May 27, 2024 at 04:50:02PM +0200, Benjamin Tissoires wrote:
>On May 27 2024, Sasha Levin wrote:
>> From: Benjamin Tissoires <bentiss@kernel.org>
>>
>> [ Upstream commit 4e6d2a297dd5be26ad409b7a05b20bd033d1c95e ]
>>
>> Duplicate of commit 0db117359e47 ("HID: add quirk for 03f0:464a HP Elite
>> Presenter Mouse"), but in a slightly better way.
>>
>> This time we actually change the application collection, making clearer
>> for userspace what the second mouse is.
>>
>> Note that having both hid-quirks fix and this HID-BPF fix is not a
>> problem at all.
>
>Please drop this patch in all backports (and FWIW, any fix in drivers/hid/bpf/progs/).
>
>HID-BPF is only available since kernel v6.3, and the compilation output
>of the in-tree file is not used directly, but shipped from udev-hid-bpf.
>
>TL;DR: this just adds noise to those stable kernel trees.

I'll drop it, thanks!

-- 
Thanks,
Sasha

^ permalink raw reply

* Re: [PATCH] HID: uclogic: avoid linking common code into multiple modules
From: Jiri Kosina @ 2024-06-19 14:30 UTC (permalink / raw)
  To: José Expósito
  Cc: Arnd Bergmann, Arnd Bergmann, Benjamin Tissoires,
	Rahul Rameshbabu, Fabio Baltieri, Ivan Gorinov, Johannes Roith,
	linux-input, linux-kernel
In-Reply-To: <ZmxuWKX9lFv3pHqG@fedora>

On Fri, 14 Jun 2024, José Expósito wrote:

> > > Turns out that, since the last time I checked the KUnit docs,
> > > we have "EXPORT_SYMBOL_IF_KUNIT" available now.
> > >
> > > I think we can use it and your final patch, without the MODULE_*
> > > changes, could look like:
> > 
> > Looks good to me, can you send that with
> > 
> > Reported-by: Arnd Bergmann <arnd@arndb.de>
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> I couldn't add the "Reported-by" tag because check-patch throws a
> warning if it is used without a "Closes" tag.

What?

Let's ignore (or even better fix) checkpatch, this is bad behavior.

> Other than that, I sent v2:
> https://lore.kernel.org/linux-input/20240614161935.230529-1-jose.exposito89@gmail.com/T/

I'll add Arnd's Reported-by: while applying that, credit must be given.

Thanks,

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* Re: [PATCH v2 1/2] HID: uclogic: Avoid linking common code into multiple modules
From: Jiri Kosina @ 2024-06-19 14:34 UTC (permalink / raw)
  To: José Expósito
  Cc: benjamin.tissoires, arnd, linux-input, linux-kernel
In-Reply-To: <20240614161935.230529-2-jose.exposito89@gmail.com>

On Fri, 14 Jun 2024, José Expósito wrote:

> The hid-uclogic-params.o and hid-uclogic-rdesc.o files are linked
> into both the driver module and the unit test, which triggers a
> W=1 warning:
> 
> scripts/Makefile.build:236: drivers/hid/Makefile: hid-uclogic-rdesc.o is added to multiple modules: hid-uclogic hid-uclogic-test
> scripts/Makefile.build:236: drivers/hid/Makefile: hid-uclogic-params.o is added to multiple modules: hid-uclogic hid-uclogic-test
> 
> Avoids this by moving these two files into a separate module
> that is used by the driver and the unit test.
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>

I've added Reported-by: for Arnd and applied, thanks!

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* Re: [PATCH v2 1/1] hid-asus: use hid for brightness control on keyboard
From: Jiri Kosina @ 2024-06-19 14:39 UTC (permalink / raw)
  To: Luke Jones; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <9e4ee526-2b94-45f3-9fe9-0f1d0918916b@app.fastmail.com>

On Sun, 16 Jun 2024, Luke Jones wrote:

> > I thought this was finalised but I'm still getting conflicting 
> > reports. Please don't merge until I confirm the fix.
> 
> This is ready for merge now. I have more confirmation that the single 
> patch with no adjustment to report_id works well.

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* Re: [PATCH v3 18/23] regulator: add s2dos05 regulator support
From: Dzmitry Sankouski @ 2024-06-19 15:49 UTC (permalink / raw)
  To: Mark Brown
  Cc: Sebastian Reichel, Bjorn Andersson, Michael Turquette,
	Stephen Boyd, Neil Armstrong, Jessica Zhang, Sam Ravnborg,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Daniel Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones, Dmitry Torokhov, Pavel Machek, Liam Girdwood,
	Uwe Kleine-König, Krzysztof Kozlowski, Konrad Dybcio,
	Chanwoo Choi, phone-devel, linux-pm, linux-kernel, linux-arm-msm,
	linux-clk, dri-devel, devicetree, linux-input, linux-leds,
	linux-pwm, linux-samsung-soc
In-Reply-To: <4eab619f-6da9-4aff-b956-69d8e2ecd47f@sirena.org.uk>

вт, 18 июн. 2024 г. в 17:08, Mark Brown <broonie@kernel.org>:
>
> On Tue, Jun 18, 2024 at 04:59:52PM +0300, Dzmitry Sankouski wrote:
>
> > index 000000000000..3c58a1bd2262
> > --- /dev/null
> > +++ b/drivers/regulator/s2dos05-regulator.c
> > @@ -0,0 +1,362 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * s2dos05.c - Regulator driver for the Samsung s2dos05
> > + *
>
> Please make the entire comment a C++ one so things look more
> intentional.
>
Do you mean enclosing the first line (license identifier) in /* */
style comment?

^ permalink raw reply

* Re: [PATCH v3 18/23] regulator: add s2dos05 regulator support
From: Mark Brown @ 2024-06-19 15:52 UTC (permalink / raw)
  To: Dzmitry Sankouski
  Cc: Sebastian Reichel, Bjorn Andersson, Michael Turquette,
	Stephen Boyd, Neil Armstrong, Jessica Zhang, Sam Ravnborg,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Daniel Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones, Dmitry Torokhov, Pavel Machek, Liam Girdwood,
	Uwe Kleine-König, Krzysztof Kozlowski, Konrad Dybcio,
	Chanwoo Choi, phone-devel, linux-pm, linux-kernel, linux-arm-msm,
	linux-clk, dri-devel, devicetree, linux-input, linux-leds,
	linux-pwm, linux-samsung-soc
In-Reply-To: <CABTCjFDUKgeYWuwo8eLt+5WD=4O+kLbFwMRxsTufGnqK0Ecpvg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 634 bytes --]

On Wed, Jun 19, 2024 at 06:49:06PM +0300, Dzmitry Sankouski wrote:
> вт, 18 июн. 2024 г. в 17:08, Mark Brown <broonie@kernel.org>:
> > On Tue, Jun 18, 2024 at 04:59:52PM +0300, Dzmitry Sankouski wrote:

> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * s2dos05.c - Regulator driver for the Samsung s2dos05
> > > + *

> > Please make the entire comment a C++ one so things look more
> > intentional.

> Do you mean enclosing the first line (license identifier) in /* */
> style comment?

No, that would be a C comment.  Please use C++ style for the rest of the
header as well as the first line.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH v5 2/2] dt-bindings: input: Goodix SPI HID Touchscreen
From: Conor Dooley @ 2024-06-19 18:09 UTC (permalink / raw)
  To: Charles Wang
  Cc: dmitry.torokhov, dan.carpenter, robh, dianders, krzk+dt, jikos,
	bentiss, hbarnor, linux-input, devicetree, linux-kernel
In-Reply-To: <ZnI_kt3APuF6NWvR@ux-UP-WHL01>

[-- Attachment #1: Type: text/plain, Size: 777 bytes --]

On Wed, Jun 19, 2024 at 10:16:50AM +0800, Charles Wang wrote:
> Hi Conor,
> 
> On Tue, Jun 18, 2024 at 06:43:18PM +0100, Conor Dooley wrote:
> > On Tue, Jun 18, 2024 at 04:44:54PM +0800, Charles Wang wrote:

> > > +  reset-gpios:
> > > +    maxItems: 1
> > > +    description: reset gpio the chip is connected to.
> > 
> > The description here isn't needed, it's obvious :)
> >
> 
> Ack.
> 
> > > +  goodix,hid-report-addr:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: |
> > 
> > This | is not needed, there's no formatting to preserve.
> > 
> > Those are nits:
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > 
> 
> Thank you. If there is a next version, I will fix these.

Sounds good, thank you!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* [PATCH 0/2] Input: ims-pcu - annotate struct ims_pcu_flash_fmt with __counted_by
From: Javier Carrasco @ 2024-06-19 21:13 UTC (permalink / raw)
  To: Dmitry Torokhov, Kees Cook, Gustavo A. R. Silva
  Cc: linux-input, linux-kernel, linux-hardening, Javier Carrasco

The length is assigned before the first reference to the data flexible
array (see ims_pcu_flash_firmware()), which allows for a straightforward
annotation without further modifications.

When at it, I also fixed an error message that repeats the word "input".

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
Javier Carrasco (2):
      Input: ims-pcu - annotate struct ims_pcu_flash_fmt with __counted_by
      Input: ims-pcu - drop repeated "input" in error message

 drivers/input/misc/ims-pcu.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
---
base-commit: 2102cb0d050d34d50b9642a3a50861787527e922
change-id: 20240619-ims-pcu-counted_by-7ff24a731073

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>


^ permalink raw reply

* [PATCH 1/2] Input: ims-pcu - annotate struct ims_pcu_flash_fmt with __counted_by
From: Javier Carrasco @ 2024-06-19 21:13 UTC (permalink / raw)
  To: Dmitry Torokhov, Kees Cook, Gustavo A. R. Silva
  Cc: linux-input, linux-kernel, linux-hardening, Javier Carrasco
In-Reply-To: <20240619-ims-pcu-counted_by-v1-0-3ee0ead2e57d@gmail.com>

Use the __counted_by compiler attribute for the data[] flexible array
member to improve the results of array bound sanitizers.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/input/misc/ims-pcu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index 408a586f8c36..91f8ad826238 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -761,7 +761,7 @@ static int ims_pcu_switch_to_bootloader(struct ims_pcu *pcu)
 struct ims_pcu_flash_fmt {
 	__le32 addr;
 	u8 len;
-	u8 data[];
+	u8 data[] __counted_by(len);
 };
 
 static unsigned int ims_pcu_count_fw_records(const struct firmware *fw)

-- 
2.40.1


^ permalink raw reply related

* [PATCH 2/2] Input: ims-pcu - drop repeated "input" in error message
From: Javier Carrasco @ 2024-06-19 21:13 UTC (permalink / raw)
  To: Dmitry Torokhov, Kees Cook, Gustavo A. R. Silva
  Cc: linux-input, linux-kernel, linux-hardening, Javier Carrasco
In-Reply-To: <20240619-ims-pcu-counted_by-v1-0-3ee0ead2e57d@gmail.com>

This case of the common error message upon failure of
input_allocate_device() repeats the word "input".

Drop one "input" from the error message.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/input/misc/ims-pcu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index 91f8ad826238..a8c474de01ad 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -207,8 +207,7 @@ static int ims_pcu_setup_buttons(struct ims_pcu *pcu,
 
 	input = input_allocate_device();
 	if (!input) {
-		dev_err(pcu->dev,
-			"Not enough memory for input input device\n");
+		dev_err(pcu->dev, "Not enough memory for input device\n");
 		return -ENOMEM;
 	}
 

-- 
2.40.1


^ permalink raw reply related

* Re: [PATCH 1/2] Input: ims-pcu - annotate struct ims_pcu_flash_fmt with __counted_by
From: Kees Cook @ 2024-06-19 21:19 UTC (permalink / raw)
  To: Javier Carrasco
  Cc: Dmitry Torokhov, Gustavo A. R. Silva, linux-input, linux-kernel,
	linux-hardening
In-Reply-To: <20240619-ims-pcu-counted_by-v1-1-3ee0ead2e57d@gmail.com>

On Wed, Jun 19, 2024 at 11:13:21PM +0200, Javier Carrasco wrote:
> Use the __counted_by compiler attribute for the data[] flexible array
> member to improve the results of array bound sanitizers.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

Looks good to me. Thanks!

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

^ permalink raw reply

* Re: [PATCH 2/2] Input: ims-pcu - drop repeated "input" in error message
From: Kees Cook @ 2024-06-19 21:19 UTC (permalink / raw)
  To: Javier Carrasco
  Cc: Dmitry Torokhov, Gustavo A. R. Silva, linux-input, linux-kernel,
	linux-hardening
In-Reply-To: <20240619-ims-pcu-counted_by-v1-2-3ee0ead2e57d@gmail.com>

On Wed, Jun 19, 2024 at 11:13:22PM +0200, Javier Carrasco wrote:
> This case of the common error message upon failure of
> input_allocate_device() repeats the word "input".
> 
> Drop one "input" from the error message.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

^ permalink raw reply

* Re: [PATCH 1/2] Input: ims-pcu - annotate struct ims_pcu_flash_fmt with __counted_by
From: Gustavo A. R. Silva @ 2024-06-19 21:26 UTC (permalink / raw)
  To: Javier Carrasco, Dmitry Torokhov, Kees Cook, Gustavo A. R. Silva
  Cc: linux-input, linux-kernel, linux-hardening
In-Reply-To: <20240619-ims-pcu-counted_by-v1-1-3ee0ead2e57d@gmail.com>



On 19/06/24 23:13, Javier Carrasco wrote:
> Use the __counted_by compiler attribute for the data[] flexible array
> member to improve the results of array bound sanitizers.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
-- 
Gustavo

> ---
>   drivers/input/misc/ims-pcu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
> index 408a586f8c36..91f8ad826238 100644
> --- a/drivers/input/misc/ims-pcu.c
> +++ b/drivers/input/misc/ims-pcu.c
> @@ -761,7 +761,7 @@ static int ims_pcu_switch_to_bootloader(struct ims_pcu *pcu)
>   struct ims_pcu_flash_fmt {
>   	__le32 addr;
>   	u8 len;
> -	u8 data[];
> +	u8 data[] __counted_by(len);
>   };
>   
>   static unsigned int ims_pcu_count_fw_records(const struct firmware *fw)
> 

^ permalink raw reply


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