Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] dt-bindings: vendor-prefixes: Add Emcraft Systems
From: Krzysztof Kozlowski @ 2024-03-28 22:00 UTC (permalink / raw)
  To: Gilles Talis, devicetree, imx, linux-arm-kernel
  Cc: conor+dt, krzysztof.kozlowski+dt, robh, shawnguo, festevam, alex
In-Reply-To: <20240328202320.187596-2-gilles.talis@gmail.com>

On 28/03/2024 21:23, Gilles Talis wrote:
> Add an entry for Emcraft Systems (https://www.emcraft.com/)
> 
> Signed-off-by: Gilles Talis <gilles.talis@gmail.com>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


---

This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH 2/3] dt-bindings: arm: Add Emcraft Systems i.MX8M Plus NavQ+ Kit
From: Krzysztof Kozlowski @ 2024-03-28 22:00 UTC (permalink / raw)
  To: Gilles Talis, devicetree, imx, linux-arm-kernel
  Cc: conor+dt, krzysztof.kozlowski+dt, robh, shawnguo, festevam, alex
In-Reply-To: <20240328202320.187596-3-gilles.talis@gmail.com>

On 28/03/2024 21:23, Gilles Talis wrote:
> Add DT compatible string for Emcraft NavQ+ kit based on
> the i.MX8M Plus SoC from NXP
> 
> Signed-off-by: Gilles Talis <gilles.talis@gmail.com>

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH 3/3] arm64: dts: freescale: Add device tree for Emcraft Systems NavQ+ Kit
From: Krzysztof Kozlowski @ 2024-03-28 22:04 UTC (permalink / raw)
  To: Gilles Talis, devicetree, imx, linux-arm-kernel
  Cc: conor+dt, krzysztof.kozlowski+dt, robh, shawnguo, festevam, alex
In-Reply-To: <20240328202320.187596-4-gilles.talis@gmail.com>

On 28/03/2024 21:23, Gilles Talis wrote:
> The Emcraft Systems NavQ+ kit is a mobile robotics platform
> based on NXP i.MX8 MPlus SoC.
> 
> The following interfaces and devices are enabled:
> - eMMC
> - Gigabit Ethernet
> - RTC
> - SD-Card
> - UART console
> 
> Signed-off-by: Gilles Talis <gilles.talis@gmail.com>
> ---
>  arch/arm64/boot/dts/freescale/Makefile        |   1 +
>  .../arm64/boot/dts/freescale/imx8mp-navqp.dts | 435 ++++++++++++++++++
>  2 files changed, 436 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-navqp.dts
> 
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index 045250d0a040..bf99864c0bc4 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -166,6 +166,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mp-dhcom-pdk3.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-msc-sm2s-ep1.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mp-navqp.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-skov-revb-hdmi.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-skov-revb-lt6.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-navqp.dts b/arch/arm64/boot/dts/freescale/imx8mp-navqp.dts
> new file mode 100644
> index 000000000000..8182e71008f8
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-navqp.dts
> @@ -0,0 +1,435 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2021 Emcraft Systems
> + * Copyright 2024 Gilles Talis <gilles.talis@gmail.com>
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include "imx8mp.dtsi"
> +
> +/ {
> +	model = "Emcraft Systems i.MX8MPlus NavQ+ Kit";
> +	compatible = "emcraft,imx8mp-navqp", "fsl,imx8mp";
> +
> +	chosen {
> +		stdout-path = &uart2;
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_gpio_led>;
> +
> +		status {

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

> +			label = "status";

Please provide color and function properties, if reasonable, and then
remove label property.

> +			gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>;
> +			default-state = "on";
> +		};
> +	};
> +

...

> +	pinctrl_i2c6: i2c6grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_UART4_RXD__I2C6_SCL				0x400001c3
> +			MX8MP_IOMUXC_UART4_TXD__I2C6_SDA				0x400001c3
> +		>;
> +	};
> +
> +	pinctrl_pmic: pmicirq {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_GPIO1_IO03__GPIO1_IO03				0x41
> +		>;
> +	};
> +
> +	pinctrl_reg_usdhc2_vmmc: regusdhc2vmmcgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19				0x41
> +		>;
> +	};
> +
> +	pinctrl_uart2: uart2grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX				0x49
> +			MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX				0x49
> +		>;
> +	};
> +
> +	pinctrl_usdhc2: usdhc2grp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK				0x190
> +			MX8MP_IOMUXC_SD2_CMD__USDHC2_CMD				0x1d0
> +			MX8MP_IOMUXC_SD2_DATA0__USDHC2_DATA0				0x1d0
> +			MX8MP_IOMUXC_SD2_DATA1__USDHC2_DATA1				0x1d0
> +			MX8MP_IOMUXC_SD2_DATA2__USDHC2_DATA2				0x1d0
> +			MX8MP_IOMUXC_SD2_DATA3__USDHC2_DATA3				0x1d0
> +			MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT				0xc1
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_100mhz: usdhc2grp-100mhz {

Hm, you upstreamed something based on downstream source. Please avoid
that. Instead take upstream, clean DTS and customize it to your needs.
Otherwise you send patch with the same issues we fixed.

Standard form letter:
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

Nodes end on 'grp' I believe.


Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 1/2] dt-bindings: thermal: amlogic: add support for A1 thermal sensor
From: Conor Dooley @ 2024-03-28 22:21 UTC (permalink / raw)
  To: Dmitry Rokosov
  Cc: neil.armstrong, jbrunet, mturquette, khilman, martin.blumenstingl,
	glaroque, rafael, daniel.lezcano, rui.zhang, lukasz.luba, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, kernel, rockosov, linux-amlogic,
	linux-pm, linux-kernel, devicetree, linux-arm-kernel
In-Reply-To: <20240328191322.17551-2-ddrokosov@salutedevices.com>

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

On Thu, Mar 28, 2024 at 10:13:10PM +0300, Dmitry Rokosov wrote:
> Provide right compatible properties for Amlogic A1 Thermal Sensor
> controller. A1 family supports only one thermal node - CPU thermal
> sensor.
> 
> Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

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

^ permalink raw reply

* Re: [PATCH 3/3] arm64: dts: freescale: Add device tree for Emcraft Systems NavQ+ Kit
From: Fabio Estevam @ 2024-03-28 22:55 UTC (permalink / raw)
  To: Gilles Talis
  Cc: devicetree, imx, linux-arm-kernel, conor+dt,
	krzysztof.kozlowski+dt, robh, shawnguo, alex
In-Reply-To: <20240328202320.187596-4-gilles.talis@gmail.com>

On Thu, Mar 28, 2024 at 5:23 PM Gilles Talis <gilles.talis@gmail.com> wrote:

> +       pinctrl_i2c6: i2c6grp {
> +               fsl,pins = <
> +                       MX8MP_IOMUXC_UART4_RXD__I2C6_SCL                                0x400001c3
> +                       MX8MP_IOMUXC_UART4_TXD__I2C6_SDA                                0x400001c3

If i2c6 is not used, you can drop its pinctrl entry.

^ permalink raw reply

* Re: [PATCH 08/23] media: i2c: imx258: Add support for 24MHz clock
From: Luigi311 @ 2024-03-28 23:03 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <11f2e7d8-fd4c-4e14-81d8-cbc2cd2442fa@luigi311.com>

On 3/28/24 11:55, Luigi311 wrote:
> On 3/28/24 02:09, Sakari Ailus wrote:
>> Hi Luigi311,
>>
>> Thank you for the patchset.
>>
>> On Wed, Mar 27, 2024 at 05:16:54PM -0600, git@luigi311.com wrote:
>>> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
>>>
>>> There's no reason why only a clock of 19.2MHz is supported.
>>> Indeed this isn't even a frequency listed in the datasheet.
>>>
>>> Add support for 24MHz as well.
>>> The PLL settings result in slightly different link frequencies,
>>> so parameterise those.
>>>
>>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
>>> Signed-off-by: Luigi311 <git@luigi311.com>
>>
>> Is Luigi311 your real name? As per
>> Documentation/process/submitting-patches.rst, anonymous (or pseudonym I'd
>> say as well) contributions are not an option.
> 
> Luigi311 is not my real name but it would be a lot easier to find me if
> it was. My real name is Luis Garcia which is a super common name so its
> actually way easier to find me and all my work using my online name of
> Luigi311. I can go ahead and swap over to Luis Garcia if required but a
> name like that would provide no value in contacting/finding me since I'm
> not famous like all the other Luis Garcia's that appear on google.
> 
>>
>>> ---
>>>  drivers/media/i2c/imx258.c | 133 +++++++++++++++++++++++++++++--------
>>>  1 file changed, 107 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
>>> index 351add1bc5d5..6ee7de079454 100644
>>> --- a/drivers/media/i2c/imx258.c
>>> +++ b/drivers/media/i2c/imx258.c
>>> @@ -76,9 +76,6 @@
>>>  #define REG_CONFIG_MIRROR_FLIP		0x03
>>>  #define REG_CONFIG_FLIP_TEST_PATTERN	0x02
>>>  
>>> -/* Input clock frequency in Hz */
>>> -#define IMX258_INPUT_CLOCK_FREQ		19200000
>>> -
>>>  struct imx258_reg {
>>>  	u16 address;
>>>  	u8 val;
>>> @@ -115,7 +112,9 @@ struct imx258_mode {
>>>  };
>>>  
>>>  /* 4208x3120 needs 1267Mbps/lane, 4 lanes */
>>> -static const struct imx258_reg mipi_data_rate_1267mbps[] = {
>>> +static const struct imx258_reg mipi_1267mbps_19_2mhz[] = {
>>> +	{ 0x0136, 0x13 },
>>> +	{ 0x0137, 0x33 },
>>>  	{ 0x0301, 0x05 },
>>>  	{ 0x0303, 0x02 },
>>>  	{ 0x0305, 0x03 },
>>> @@ -133,7 +132,29 @@ static const struct imx258_reg mipi_data_rate_1267mbps[] = {
>>>  	{ 0x0823, 0xCC },
>>>  };
>>>  
>>> -static const struct imx258_reg mipi_data_rate_640mbps[] = {
>>> +static const struct imx258_reg mipi_1272mbps_24mhz[] = {
>>> +	{ 0x0136, 0x18 },
>>> +	{ 0x0137, 0x00 },
>>> +	{ 0x0301, 0x05 },
>>> +	{ 0x0303, 0x02 },
>>> +	{ 0x0305, 0x04 },
>>> +	{ 0x0306, 0x00 },
>>> +	{ 0x0307, 0xD4 },
>>> +	{ 0x0309, 0x0A },
>>> +	{ 0x030B, 0x01 },
>>> +	{ 0x030D, 0x02 },
>>> +	{ 0x030E, 0x00 },
>>> +	{ 0x030F, 0xD8 },
>>> +	{ 0x0310, 0x00 },
>>> +	{ 0x0820, 0x13 },
>>> +	{ 0x0821, 0x4C },
>>> +	{ 0x0822, 0xCC },
>>> +	{ 0x0823, 0xCC },
>>> +};
>>> +
>>> +static const struct imx258_reg mipi_640mbps_19_2mhz[] = {
>>> +	{ 0x0136, 0x13 },
>>> +	{ 0x0137, 0x33 },
>>>  	{ 0x0301, 0x05 },
>>>  	{ 0x0303, 0x02 },
>>>  	{ 0x0305, 0x03 },
>>> @@ -151,9 +172,27 @@ static const struct imx258_reg mipi_data_rate_640mbps[] = {
>>>  	{ 0x0823, 0x00 },
>>>  };
>>>  
>>> +static const struct imx258_reg mipi_642mbps_24mhz[] = {
>>> +	{ 0x0136, 0x18 },
>>> +	{ 0x0137, 0x00 },
>>> +	{ 0x0301, 0x05 },
>>> +	{ 0x0303, 0x02 },
>>> +	{ 0x0305, 0x04 },
>>> +	{ 0x0306, 0x00 },
>>> +	{ 0x0307, 0x6B },
>>> +	{ 0x0309, 0x0A },
>>> +	{ 0x030B, 0x01 },
>>> +	{ 0x030D, 0x02 },
>>> +	{ 0x030E, 0x00 },
>>> +	{ 0x030F, 0xD8 },
>>> +	{ 0x0310, 0x00 },
>>> +	{ 0x0820, 0x0A },
>>> +	{ 0x0821, 0x00 },
>>> +	{ 0x0822, 0x00 },
>>> +	{ 0x0823, 0x00 },
>>> +};
>>> +
>>>  static const struct imx258_reg mode_common_regs[] = {
>>> -	{ 0x0136, 0x13 },
>>> -	{ 0x0137, 0x33 },
>>>  	{ 0x3051, 0x00 },
>>>  	{ 0x3052, 0x00 },
>>>  	{ 0x4E21, 0x14 },
>>> @@ -313,10 +352,6 @@ static const char * const imx258_supply_name[] = {
>>>  
>>>  #define IMX258_NUM_SUPPLIES ARRAY_SIZE(imx258_supply_name)
>>>  
>>> -/* Configurations for supported link frequencies */
>>> -#define IMX258_LINK_FREQ_634MHZ	633600000ULL
>>> -#define IMX258_LINK_FREQ_320MHZ	320000000ULL
>>> -
>>>  enum {
>>>  	IMX258_LINK_FREQ_1267MBPS,
>>>  	IMX258_LINK_FREQ_640MBPS,
>>> @@ -335,25 +370,55 @@ static u64 link_freq_to_pixel_rate(u64 f)
>>>  }
>>>  
>>>  /* Menu items for LINK_FREQ V4L2 control */
>>> -static const s64 link_freq_menu_items[] = {
>>> +/* Configurations for supported link frequencies */
>>> +#define IMX258_LINK_FREQ_634MHZ	633600000ULL
>>> +#define IMX258_LINK_FREQ_320MHZ	320000000ULL
>>> +
>>> +static const s64 link_freq_menu_items_19_2[] = {
>>>  	IMX258_LINK_FREQ_634MHZ,
>>>  	IMX258_LINK_FREQ_320MHZ,
>>>  };
>>>  
>>> +/* Configurations for supported link frequencies */
>>> +#define IMX258_LINK_FREQ_636MHZ	636000000ULL
>>> +#define IMX258_LINK_FREQ_321MHZ	321000000ULL
>>
>> These values aren't used outside the array below and the macro names are
>> imprecise anyway. Could you put the numerical values to the array instead?
> 
> Ok I've removed the defines and just threw the values into the array instead.
> 
>>
>>> +
>>> +static const s64 link_freq_menu_items_24[] = {
>>> +	IMX258_LINK_FREQ_636MHZ,
>>> +	IMX258_LINK_FREQ_321MHZ,
>>> +};
>>> +
>>>  /* Link frequency configs */
>>> -static const struct imx258_link_freq_config link_freq_configs[] = {
>>> +static const struct imx258_link_freq_config link_freq_configs_19_2[] = {
>>>  	[IMX258_LINK_FREQ_1267MBPS] = {
>>>  		.pixels_per_line = IMX258_PPL_DEFAULT,
>>>  		.reg_list = {
>>> -			.num_of_regs = ARRAY_SIZE(mipi_data_rate_1267mbps),
>>> -			.regs = mipi_data_rate_1267mbps,
>>> +			.num_of_regs = ARRAY_SIZE(mipi_1267mbps_19_2mhz),
>>> +			.regs = mipi_1267mbps_19_2mhz,
>>>  		}
>>>  	},
>>>  	[IMX258_LINK_FREQ_640MBPS] = {
>>>  		.pixels_per_line = IMX258_PPL_DEFAULT,
>>>  		.reg_list = {
>>> -			.num_of_regs = ARRAY_SIZE(mipi_data_rate_640mbps),
>>> -			.regs = mipi_data_rate_640mbps,
>>> +			.num_of_regs = ARRAY_SIZE(mipi_640mbps_19_2mhz),
>>> +			.regs = mipi_640mbps_19_2mhz,
>>> +		}
>>> +	},
>>> +};
>>> +
>>> +static const struct imx258_link_freq_config link_freq_configs_24[] = {
>>> +	[IMX258_LINK_FREQ_1267MBPS] = {
>>> +		.pixels_per_line = IMX258_PPL_DEFAULT,
>>> +		.reg_list = {
>>> +			.num_of_regs = ARRAY_SIZE(mipi_1272mbps_24mhz),
>>> +			.regs = mipi_1272mbps_24mhz,
>>> +		}
>>> +	},
>>> +	[IMX258_LINK_FREQ_640MBPS] = {
>>> +		.pixels_per_line = IMX258_PPL_DEFAULT,
>>> +		.reg_list = {
>>> +			.num_of_regs = ARRAY_SIZE(mipi_642mbps_24mhz),
>>> +			.regs = mipi_642mbps_24mhz,
>>>  		}
>>>  	},
>>>  };
>>> @@ -410,6 +475,9 @@ struct imx258 {
>>>  	/* Current mode */
>>>  	const struct imx258_mode *cur_mode;
>>>  
>>> +	const struct imx258_link_freq_config *link_freq_configs;
>>> +	const s64 *link_freq_menu_items;
>>> +
>>>  	/*
>>>  	 * Mutex for serialized access:
>>>  	 * Protect sensor module set pad format and start/stop streaming safely.
>>> @@ -713,7 +781,7 @@ static int imx258_set_pad_format(struct v4l2_subdev *sd,
>>>  		imx258->cur_mode = mode;
>>>  		__v4l2_ctrl_s_ctrl(imx258->link_freq, mode->link_freq_index);
>>>  
>>> -		link_freq = link_freq_menu_items[mode->link_freq_index];
>>> +		link_freq = imx258->link_freq_menu_items[mode->link_freq_index];
>>>  		pixel_rate = link_freq_to_pixel_rate(link_freq);
>>>  		__v4l2_ctrl_s_ctrl_int64(imx258->pixel_rate, pixel_rate);
>>>  		/* Update limits and set FPS to default */
>>> @@ -727,7 +795,7 @@ static int imx258_set_pad_format(struct v4l2_subdev *sd,
>>>  			vblank_def);
>>>  		__v4l2_ctrl_s_ctrl(imx258->vblank, vblank_def);
>>>  		h_blank =
>>> -			link_freq_configs[mode->link_freq_index].pixels_per_line
>>> +			imx258->link_freq_configs[mode->link_freq_index].pixels_per_line
>>>  			 - imx258->cur_mode->width;
>>>  		__v4l2_ctrl_modify_range(imx258->hblank, h_blank,
>>>  					 h_blank, 1, h_blank);
>>> @@ -747,7 +815,7 @@ static int imx258_start_streaming(struct imx258 *imx258)
>>>  
>>>  	/* Setup PLL */
>>>  	link_freq_index = imx258->cur_mode->link_freq_index;
>>> -	reg_list = &link_freq_configs[link_freq_index].reg_list;
>>> +	reg_list = &imx258->link_freq_configs[link_freq_index].reg_list;
>>>  	ret = imx258_write_regs(imx258, reg_list->regs, reg_list->num_of_regs);
>>>  	if (ret) {
>>>  		dev_err(&client->dev, "%s failed to set plls\n", __func__);
>>> @@ -946,9 +1014,9 @@ static int imx258_init_controls(struct imx258 *imx258)
>>>  	imx258->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
>>>  				&imx258_ctrl_ops,
>>>  				V4L2_CID_LINK_FREQ,
>>> -				ARRAY_SIZE(link_freq_menu_items) - 1,
>>> +				ARRAY_SIZE(link_freq_menu_items_19_2) - 1,
>>>  				0,
>>> -				link_freq_menu_items);
>>> +				imx258->link_freq_menu_items);
>>>  
>>>  	if (imx258->link_freq)
>>>  		imx258->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>>> @@ -964,8 +1032,10 @@ static int imx258_init_controls(struct imx258 *imx258)
>>>  	if (vflip)
>>>  		vflip->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>>>  
>>> -	pixel_rate_max = link_freq_to_pixel_rate(link_freq_menu_items[0]);
>>> -	pixel_rate_min = link_freq_to_pixel_rate(link_freq_menu_items[1]);
>>> +	pixel_rate_max =
>>> +		link_freq_to_pixel_rate(imx258->link_freq_menu_items[0]);
>>> +	pixel_rate_min =
>>> +		link_freq_to_pixel_rate(imx258->link_freq_menu_items[1]);
>>
>> The arrays currently have two entries so this works but it'd nice to have a
>> bit more robust way to handle differences between the two arrays. Could you
>> maintain e.g. the number of entries in the array in a struct field perhaps?
> 
> Would it make more sense to do something like default to index 0 and then use 
> ARRAY_SIZE to iterate through the array and do a comparison to get the min and
> max size so it would always choose the correct value no matter how many entries
> there are?
> 

Actually down the patch series 15/23 set pixel_rate range to the same as the value,
changes the logic and removes those two lines all together

>>
>>>  	/* By default, PIXEL_RATE is read only */
>>>  	imx258->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx258_ctrl_ops,
>>>  				V4L2_CID_PIXEL_RATE,
>>> @@ -1086,8 +1156,19 @@ static int imx258_probe(struct i2c_client *client)
>>>  	} else {
>>>  		val = clk_get_rate(imx258->clk);
>>>  	}
>>> -	if (val != IMX258_INPUT_CLOCK_FREQ) {
>>> -		dev_err(&client->dev, "input clock frequency not supported\n");
>>> +
>>> +	switch (val) {
>>> +	case 19200000:
>>> +		imx258->link_freq_configs = link_freq_configs_19_2;
>>> +		imx258->link_freq_menu_items = link_freq_menu_items_19_2;
>>> +		break;
>>> +	case 24000000:
>>> +		imx258->link_freq_configs = link_freq_configs_24;
>>> +		imx258->link_freq_menu_items = link_freq_menu_items_24;
>>> +		break;
>>> +	default:
>>> +		dev_err(&client->dev, "input clock frequency of %u not supported\n",
>>> +			val);
>>>  		return -EINVAL;
>>>  	}
>>>  
>>
> 


^ permalink raw reply

* [PATCH 2/2] v2 arm64: dts: qcom: Add support for Samsung Galaxy Z Fold5
From: Alexandru Marc Serdeliuc via B4 Relay @ 2024-03-28 23:08 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Alexandru Marc Serdeliuc
In-Reply-To: <20240329-v2-dts-add-support-for-samsung-galaxy-zfold5-v1-0-9a91e635cacc@yahoo.com>

From: Alexandru Marc Serdeliuc <serdeliuk@yahoo.com>

Add support for Samsung Galaxy Z Fold5 (q5q) foldable phone

Currently working features:
- Framebuffer
- UFS
- i2c
- Buttons

Signed-off-by: Alexandru Marc Serdeliuc <serdeliuk@yahoo.com>
---
 Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 66beaac60e1d..dea2a23b8fc2 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -1003,6 +1003,7 @@ properties:
               - qcom,sm8550-hdk
               - qcom,sm8550-mtp
               - qcom,sm8550-qrd
+              - samsung,q5q
           - const: qcom,sm8550
 
       - items:

-- 
2.34.1



^ permalink raw reply related

* [PATCH 0/2] V2 arm64: dts: qcom: Add support for Samsung Galaxy Z Fold5
From: Alexandru Marc Serdeliuc via B4 Relay @ 2024-03-28 23:08 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Alexandru Marc Serdeliuc

Added the  binding file to  the  same patch that includes the  device tree.
Refactored the  decivetree
- removed bootargs
- removed unused nodes
- removed disabled nodes
- added missing identation
- removed all the comments at the end of the nodes
- moved node status at the end of the nodes
- ordered all the nodes alphabetically
- ran the recommended dtbs tests all returned success

Signed-off-by: Alexandru Marc Serdeliuc <serdeliuk@yahoo.com>
---
Alexandru Marc Serdeliuc (2):
      V2 arm64: dts: qcom: Add support for Samsung Galaxy Z Fold5
      v2 arm64: dts: qcom: Add support for Samsung Galaxy Z Fold5

 Documentation/devicetree/bindings/arm/qcom.yaml |   1 +
 arch/arm64/boot/dts/qcom/Makefile               |   1 +
 arch/arm64/boot/dts/qcom/sm8550-samsung-q5q.dts | 584 ++++++++++++++++++++++++
 3 files changed, 586 insertions(+)
---
base-commit: 4cece764965020c22cff7665b18a012006359095
change-id: 20240328-v2-dts-add-support-for-samsung-galaxy-zfold5-44b90138f3be

Best regards,
-- 
Alexandru Marc Serdeliuc <serdeliuk@yahoo.com>



^ permalink raw reply

* [PATCH 1/2] V2 arm64: dts: qcom: Add support for Samsung Galaxy Z Fold5
From: Alexandru Marc Serdeliuc via B4 Relay @ 2024-03-28 23:08 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Alexandru Marc Serdeliuc
In-Reply-To: <20240329-v2-dts-add-support-for-samsung-galaxy-zfold5-v1-0-9a91e635cacc@yahoo.com>

From: Alexandru Marc Serdeliuc <serdeliuk@yahoo.com>

Add support for Samsung Galaxy Z Fold5 (q5q) foldable phone

Currently working features:
- Framebuffer
- UFS
- i2c
- Buttons

Signed-off-by: Alexandru Marc Serdeliuc <serdeliuk@yahoo.com>
---
 arch/arm64/boot/dts/qcom/Makefile               |   1 +
 arch/arm64/boot/dts/qcom/sm8550-samsung-q5q.dts | 584 ++++++++++++++++++++++++
 2 files changed, 585 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 7d40ec5e7d21..a7503fd35b6c 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -241,6 +241,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sm8450-sony-xperia-nagara-pdx224.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8550-hdk.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8550-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8550-qrd.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sm8550-samsung-q5q.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8650-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8650-qrd.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= x1e80100-crd.dtb
diff --git a/arch/arm64/boot/dts/qcom/sm8550-samsung-q5q.dts b/arch/arm64/boot/dts/qcom/sm8550-samsung-q5q.dts
new file mode 100644
index 000000000000..aec375fc32b4
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sm8550-samsung-q5q.dts
@@ -0,0 +1,584 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2024, Alexandru Marc Serdeliuc <serdeliuk@yahoo.com>
+ * Copyright (c) 2024, David Wronek <david@mainlining.org>
+ * Copyright (c) 2022, Linaro Limited
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+#include "sm8550.dtsi"
+#include "pm8550.dtsi"
+#include "pm8550vs.dtsi"
+#include "pmk8550.dtsi"
+
+/ {
+	model = "Samsung Galaxy Z Fold5";
+	compatible = "samsung,q5q", "qcom,sm8550";
+	#address-cells = <0x02>;
+	#size-cells = <0x02>;
+	chassis-type = "handset";
+
+	aliases {
+		serial0 = &uart7;
+	};
+
+	chosen {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		framebuffer: framebuffer@b8000000 {
+			compatible = "simple-framebuffer";
+			reg = <0x0 0xb8000000 0x0 0x2b00000>;
+			width = <2176>;
+			height = <1812>;
+			stride = <(2176 * 4)>;
+			format = "a8r8g8b8";
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+		pinctrl-0 = <&volume_up_n>;
+		pinctrl-names = "default";
+
+		key-volume-up {
+			label = "Volume Up";
+			linux,code = <KEY_VOLUMEUP>;
+			gpios = <&pm8550_gpios 6 GPIO_ACTIVE_LOW>;
+			debounce-interval = <15>;
+			linux,can-disable;
+			wakeup-source;
+		};
+	};
+
+	vph_pwr: vph-pwr-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vph_pwr";
+		regulator-min-microvolt = <3700000>;
+		regulator-max-microvolt = <3700000>;
+		regulator-always-on;
+		regulator-boot-on;
+	};
+
+	reserved-memory {
+		/*
+		 * The bootloader will only keep display hardware enabled
+		 * if this memory region is named exactly 'splash_region'
+		 */
+		splash_region@b8000000 {
+			reg = <0x0 0xb8000000 0x0 0x2b00000>;
+			no-map;
+		};
+	};
+};
+
+&apps_rsc {
+	regulators-0 {
+		compatible = "qcom,pm8550-rpmh-regulators";
+		qcom,pmic-id = "b";
+
+		vreg_bob1: bob1 {
+			regulator-name = "vreg_bob1";
+			regulator-min-microvolt = <3296000>;
+			regulator-max-microvolt = <3960000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_bob2: bob2 {
+			regulator-name = "vreg_bob2";
+			regulator-min-microvolt = <2720000>;
+			regulator-max-microvolt = <3960000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l1b_1p8: ldo1 {
+			regulator-name = "vreg_l1b_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l2b_3p0: ldo2 {
+			regulator-name = "vreg_l2b_3p0";
+			regulator-min-microvolt = <3008000>;
+			regulator-max-microvolt = <3008000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l5b_3p1: ldo5 {
+			regulator-name = "vreg_l5b_3p1";
+			regulator-min-microvolt = <3104000>;
+			regulator-max-microvolt = <3104000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l6b_1p8: ldo6 {
+			regulator-name = "vreg_l6b_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3008000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l7b_1p8: ldo7 {
+			regulator-name = "vreg_l7b_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3008000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l8b_1p8: ldo8 {
+			regulator-name = "vreg_l8b_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3008000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l9b_2p9: ldo9 {
+			regulator-name = "vreg_l9b_2p9";
+			regulator-min-microvolt = <2960000>;
+			regulator-max-microvolt = <3008000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l11b_1p2: ldo11 {
+			regulator-name = "vreg_l11b_1p2";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1504000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l12b_1p8: ldo12 {
+			regulator-name = "vreg_l12b_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l13b_3p0: ldo13 {
+			regulator-name = "vreg_l13b_3p0";
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3000000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l14b_3p2: ldo14 {
+			regulator-name = "vreg_l14b_3p2";
+			regulator-min-microvolt = <3200000>;
+			regulator-max-microvolt = <3200000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l15b_1p8: ldo15 {
+			regulator-name = "vreg_l15b_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+			regulator-allow-set-load;
+			regulator-allowed-modes = <RPMH_REGULATOR_MODE_LPM
+						   RPMH_REGULATOR_MODE_HPM>;
+			regulator-always-on;
+		};
+
+		vreg_l16b_2p8: ldo16 {
+			regulator-name = "vreg_l16b_2p8";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l17b_2p5: ldo17 {
+			regulator-name = "vreg_l17b_2p5";
+			regulator-min-microvolt = <2504000>;
+			regulator-max-microvolt = <2504000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+	};
+
+	regulators-1 {
+		compatible = "qcom,pm8550vs-rpmh-regulators";
+		qcom,pmic-id = "c";
+
+		vreg_l3c_0p91: ldo3 {
+			regulator-name = "vreg_l3c_0p9";
+			regulator-min-microvolt = <880000>;
+			regulator-max-microvolt = <912000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+	};
+
+	regulators-2 {
+		compatible = "qcom,pm8550vs-rpmh-regulators";
+		qcom,pmic-id = "d";
+
+		vreg_l1d_0p88: ldo1 {
+			regulator-name = "vreg_l1d_0p88";
+			regulator-min-microvolt = <880000>;
+			regulator-max-microvolt = <920000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+	};
+
+	regulators-3 {
+		compatible = "qcom,pm8550vs-rpmh-regulators";
+		qcom,pmic-id = "e";
+
+		vreg_s4e_0p9: smps4 {
+			regulator-name = "vreg_s4e_0p9";
+			regulator-min-microvolt = <904000>;
+			regulator-max-microvolt = <984000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_s5e_1p1: smps5 {
+			regulator-name = "vreg_s5e_1p1";
+			regulator-min-microvolt = <1080000>;
+			regulator-max-microvolt = <1120000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l1e_0p88: ldo1 {
+			regulator-name = "vreg_l1e_0p88";
+			regulator-min-microvolt = <880000>;
+			regulator-max-microvolt = <880000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l2e_0p9: ldo2 {
+			regulator-name = "vreg_l2e_0p9";
+			regulator-min-microvolt = <904000>;
+			regulator-max-microvolt = <970000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l3e_1p2: ldo3 {
+			regulator-name = "vreg_l3e_1p2";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+	};
+
+	regulators-4 {
+		compatible = "qcom,pm8550ve-rpmh-regulators";
+		qcom,pmic-id = "f";
+
+		vreg_s4f_0p5: smps4 {
+			regulator-name = "vreg_s4f_0p5";
+			regulator-min-microvolt = <500000>;
+			regulator-max-microvolt = <700000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l1f_0p9: ldo1 {
+			regulator-name = "vreg_l1f_0p9";
+			regulator-min-microvolt = <912000>;
+			regulator-max-microvolt = <912000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l2f_0p88: ldo2 {
+			regulator-name = "vreg_l2f_0p88";
+			regulator-min-microvolt = <880000>;
+			regulator-max-microvolt = <912000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l3f_0p91: ldo3 {
+			regulator-name = "vreg_l3f_0p91";
+			regulator-min-microvolt = <880000>;
+			regulator-max-microvolt = <912000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+	};
+
+	regulators-5 {
+		compatible = "qcom,pm8550vs-rpmh-regulators";
+		qcom,pmic-id = "g";
+
+		vreg_s1g_1p2: smps1 {
+			regulator-name = "vreg_s1g_1p2";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1300000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_s2g_0p8: smps2 {
+			regulator-name = "vreg_s2g_0p8";
+			regulator-min-microvolt = <800000>;
+			regulator-max-microvolt = <1000000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_s3g_0p7: smps3 {
+			regulator-name = "vreg_s3g_0p7";
+			regulator-min-microvolt = <300000>;
+			regulator-max-microvolt = <1004000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_s4g_1p3: smps4 {
+			regulator-name = "vreg_s4g_1p3";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1352000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_s5g_0p8: smps5 {
+			regulator-name = "vreg_s5g_0p8";
+			regulator-min-microvolt = <500000>;
+			regulator-max-microvolt = <1004000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_s6g_1p8: smps6 {
+			regulator-name = "vreg_s6g_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2000000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l1g_1p2: ldo1 {
+			regulator-name = "vreg_l1g_1p2";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l2g_1p2: ldo2 {
+			regulator-name = "vreg_l2g_1p2";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l3g_1p2: ldo3 {
+			regulator-name = "vreg_l3g_1p2";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+	};
+
+	regulators-6 {
+		compatible = "qcom,pm8010-rpmh-regulators";
+		qcom,pmic-id = "m";
+
+		vreg_l1m_1p056: ldo1 {
+			regulator-name = "vreg_l1m_1p056";
+			regulator-min-microvolt = <1056000>;
+			regulator-max-microvolt = <1056000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l2m_1p056: ldo2 {
+			regulator-name = "vreg_l2m_1p056";
+			regulator-min-microvolt = <1056000>;
+			regulator-max-microvolt = <1056000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l3m_2p8: ldo3 {
+			regulator-name = "vreg_l3m_2p8";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l4m_2p8: ldo4 {
+			regulator-name = "vreg_l4m_2p8";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l5m_1p8: ldo5 {
+			regulator-name = "vreg_l5m_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l6m_1p8: ldo6 {
+			regulator-name = "vreg_l6m_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l7m_2p9: ldo7 {
+			regulator-name = "vreg_l7m_2p9";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2904000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+	};
+
+	regulators-7 {
+		compatible = "qcom,pm8010-rpmh-regulators";
+		qcom,pmic-id = "n";
+
+		vreg_l1n_1p1: ldo1 {
+			regulator-name = "vreg_l1n_1p1";
+			regulator-min-microvolt = <1104000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l2n_1p1: ldo2 {
+			regulator-name = "vreg_l2n_1p1";
+			regulator-min-microvolt = <1104000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l3n_2p8: ldo3 {
+			regulator-name = "vreg_l3n_2p8";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <3000000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l4n_2p8: ldo4 {
+			regulator-name = "vreg_l4n_2p8";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l5n_1p8: ldo5 {
+			regulator-name = "vreg_l5n_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l6n_3p3: ldo6 {
+			regulator-name = "vreg_l6n_3p3";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <3304000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l7n_2p96: ldo7 {
+			regulator-name = "vreg_l7n_2p96";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2960000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+	};
+};
+
+
+&dispcc {
+	status = "disabled";
+};
+
+&i2c_master_hub_0 {
+	status = "okay";
+};
+
+&pcie_1_phy_aux_clk {
+	clock-frequency = <1000>;
+};
+
+&pcie0 {
+	wake-gpios = <&tlmm 96 GPIO_ACTIVE_HIGH>;
+	perst-gpios = <&tlmm 94 GPIO_ACTIVE_LOW>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie0_default_state>;
+	status = "okay";
+};
+
+&pcie0_phy {
+	vdda-phy-supply = <&vreg_l1e_0p88>;
+	vdda-pll-supply = <&vreg_l3e_1p2>;
+	status = "okay";
+};
+
+&pcie1 {
+	wake-gpios = <&tlmm 99 GPIO_ACTIVE_HIGH>;
+	perst-gpios = <&tlmm 97 GPIO_ACTIVE_LOW>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie1_default_state>;
+	status = "okay";
+};
+
+&pcie1_phy {
+	vdda-phy-supply = <&vreg_l3c_0p91>;
+	vdda-pll-supply = <&vreg_l3e_1p2>;
+	vdda-qref-supply = <&vreg_l1e_0p88>;
+	status = "okay";
+};
+
+&pm8550_gpios {
+	volume_up_n: volume-up-n-state {
+		pins = "gpio6";
+		function = "normal";
+		power-source = <1>;
+		bias-pull-up;
+		input-enable;
+	};
+};
+
+&pon_pwrkey {
+	status = "okay";
+};
+
+&pon_resin {
+	status = "okay";
+	linux,code = <KEY_VOLUMEDOWN>;
+};
+
+&qupv3_id_0 {
+	status = "okay";
+};
+
+&remoteproc_adsp {
+	firmware-name = "qcom/sm8550/adsp.mbn",
+			"qcom/sm8550/adsp_dtb.mbn";
+	status = "okay";
+};
+
+&remoteproc_cdsp {
+	firmware-name = "qcom/sm8550/cdsp.mbn",
+			"qcom/sm8550/cdsp_dtb.mbn";
+	status = "okay";
+};
+
+&remoteproc_mpss {
+	firmware-name = "qcom/sm8550/modem.mbn",
+			"qcom/sm8550/modem_dtb.mbn";
+	status = "okay";
+};
+
+&sleep_clk {
+	clock-frequency = <32000>;
+};
+
+&tlmm {
+	gpio-reserved-ranges = <36 4>, <50 2>;
+};
+
+&ufs_mem_hc {
+	reset-gpios = <&tlmm 210 GPIO_ACTIVE_LOW>;
+	vcc-supply = <&vreg_l17b_2p5>;
+	vcc-max-microamp = <1300000>;
+	vccq-supply = <&vreg_l1g_1p2>;
+	vccq-max-microamp = <1200000>;
+	vdd-hba-supply = <&vreg_l3g_1p2>;
+	status = "okay";
+};
+
+&ufs_mem_phy {
+	vdda-phy-supply = <&vreg_l1d_0p88>;
+	vdda-pll-supply = <&vreg_l3e_1p2>;
+	status = "okay";
+};
+
+&xo_board {
+	clock-frequency = <76800000>;
+};

-- 
2.34.1



^ permalink raw reply related

* Re: [PATCH 09/23] media: i2c: imx258: Add support for running on 2 CSI data lanes
From: Luigi311 @ 2024-03-28 23:42 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <ZgUoH4mhl3Ol280r@kekkonen.localdomain>

On 3/28/24 02:19, Sakari Ailus wrote:
> Hi Luigi311, Dave,
> 
> On Wed, Mar 27, 2024 at 05:16:55PM -0600, git@luigi311.com wrote:
>> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
>>
>> Extends the driver to also support 2 data lanes.
>> Frame rates are obviously more restricted on 2 lanes, but some
>> hardware simply hasn't wired more up.
>>
>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
>> Signed-off-by: Luigi311 <git@luigi311.com>
>> ---
>>  drivers/media/i2c/imx258.c | 214 ++++++++++++++++++++++++++++++++-----
>>  1 file changed, 190 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
>> index 6ee7de079454..c65b9aad3b0a 100644
>> --- a/drivers/media/i2c/imx258.c
>> +++ b/drivers/media/i2c/imx258.c
>> @@ -86,12 +86,18 @@ struct imx258_reg_list {
>>  	const struct imx258_reg *regs;
>>  };
>>  
>> +enum {
>> +	IMX258_2_LANE_MODE,
>> +	IMX258_4_LANE_MODE,
>> +	IMX258_LANE_CONFIGS,
>> +};
>> +
>>  /* Link frequency config */
>>  struct imx258_link_freq_config {
>>  	u32 pixels_per_line;
>>  
>>  	/* PLL registers for this link frequency */
>> -	struct imx258_reg_list reg_list;
>> +	struct imx258_reg_list reg_list[IMX258_LANE_CONFIGS];
>>  };
>>  
>>  /* Mode : resolution and related config&values */
>> @@ -111,8 +117,34 @@ struct imx258_mode {
>>  	struct imx258_reg_list reg_list;
>>  };
>>  
>> -/* 4208x3120 needs 1267Mbps/lane, 4 lanes */
>> -static const struct imx258_reg mipi_1267mbps_19_2mhz[] = {
>> +/*
>> + * 4208x3120 @ 30 fps needs 1267Mbps/lane, 4 lanes.
>> + * To avoid further computation of clock settings, adopt the same per
>> + * lane data rate when using 2 lanes, thus allowing a maximum of 15fps.
>> + */
>> +static const struct imx258_reg mipi_1267mbps_19_2mhz_2l[] = {
>> +	{ 0x0136, 0x13 },
>> +	{ 0x0137, 0x33 },
>> +	{ 0x0301, 0x0A },
>> +	{ 0x0303, 0x02 },
>> +	{ 0x0305, 0x03 },
>> +	{ 0x0306, 0x00 },
>> +	{ 0x0307, 0xC6 },
>> +	{ 0x0309, 0x0A },
>> +	{ 0x030B, 0x01 },
>> +	{ 0x030D, 0x02 },
>> +	{ 0x030E, 0x00 },
>> +	{ 0x030F, 0xD8 },
>> +	{ 0x0310, 0x00 },
>> +
>> +	{ 0x0114, 0x01 },
>> +	{ 0x0820, 0x09 },
>> +	{ 0x0821, 0xa6 },
>> +	{ 0x0822, 0x66 },
>> +	{ 0x0823, 0x66 },
>> +};
>> +
>> +static const struct imx258_reg mipi_1267mbps_19_2mhz_4l[] = {
>>  	{ 0x0136, 0x13 },
>>  	{ 0x0137, 0x33 },
>>  	{ 0x0301, 0x05 },
>> @@ -126,16 +158,18 @@ static const struct imx258_reg mipi_1267mbps_19_2mhz[] = {
>>  	{ 0x030E, 0x00 },
>>  	{ 0x030F, 0xD8 },
>>  	{ 0x0310, 0x00 },
>> +
>> +	{ 0x0114, 0x03 },
>>  	{ 0x0820, 0x13 },
>>  	{ 0x0821, 0x4C },
>>  	{ 0x0822, 0xCC },
>>  	{ 0x0823, 0xCC },
>>  };
>>  
>> -static const struct imx258_reg mipi_1272mbps_24mhz[] = {
>> +static const struct imx258_reg mipi_1272mbps_24mhz_2l[] = {
>>  	{ 0x0136, 0x18 },
>>  	{ 0x0137, 0x00 },
>> -	{ 0x0301, 0x05 },
>> +	{ 0x0301, 0x0a },
>>  	{ 0x0303, 0x02 },
>>  	{ 0x0305, 0x04 },
>>  	{ 0x0306, 0x00 },
>> @@ -146,13 +180,59 @@ static const struct imx258_reg mipi_1272mbps_24mhz[] = {
>>  	{ 0x030E, 0x00 },
>>  	{ 0x030F, 0xD8 },
>>  	{ 0x0310, 0x00 },
>> +
>> +	{ 0x0114, 0x01 },
>>  	{ 0x0820, 0x13 },
>>  	{ 0x0821, 0x4C },
>>  	{ 0x0822, 0xCC },
>>  	{ 0x0823, 0xCC },
>>  };
>>  
>> -static const struct imx258_reg mipi_640mbps_19_2mhz[] = {
>> +static const struct imx258_reg mipi_1272mbps_24mhz_4l[] = {
>> +	{ 0x0136, 0x18 },
>> +	{ 0x0137, 0x00 },
>> +	{ 0x0301, 0x05 },
>> +	{ 0x0303, 0x02 },
>> +	{ 0x0305, 0x04 },
>> +	{ 0x0306, 0x00 },
>> +	{ 0x0307, 0xD4 },
>> +	{ 0x0309, 0x0A },
>> +	{ 0x030B, 0x01 },
>> +	{ 0x030D, 0x02 },
>> +	{ 0x030E, 0x00 },
>> +	{ 0x030F, 0xD8 },
>> +	{ 0x0310, 0x00 },
>> +
>> +	{ 0x0114, 0x03 },
>> +	{ 0x0820, 0x13 },
>> +	{ 0x0821, 0xE0 },
>> +	{ 0x0822, 0x00 },
>> +	{ 0x0823, 0x00 },
>> +};
>> +
>> +static const struct imx258_reg mipi_640mbps_19_2mhz_2l[] = {
>> +	{ 0x0136, 0x13 },
>> +	{ 0x0137, 0x33 },
>> +	{ 0x0301, 0x05 },
>> +	{ 0x0303, 0x02 },
>> +	{ 0x0305, 0x03 },
>> +	{ 0x0306, 0x00 },
>> +	{ 0x0307, 0x64 },
>> +	{ 0x0309, 0x0A },
>> +	{ 0x030B, 0x01 },
>> +	{ 0x030D, 0x02 },
>> +	{ 0x030E, 0x00 },
>> +	{ 0x030F, 0xD8 },
>> +	{ 0x0310, 0x00 },
>> +
>> +	{ 0x0114, 0x01 },
>> +	{ 0x0820, 0x05 },
>> +	{ 0x0821, 0x00 },
>> +	{ 0x0822, 0x00 },
>> +	{ 0x0823, 0x00 },
>> +};
>> +
>> +static const struct imx258_reg mipi_640mbps_19_2mhz_4l[] = {
>>  	{ 0x0136, 0x13 },
>>  	{ 0x0137, 0x33 },
>>  	{ 0x0301, 0x05 },
>> @@ -166,13 +246,37 @@ static const struct imx258_reg mipi_640mbps_19_2mhz[] = {
>>  	{ 0x030E, 0x00 },
>>  	{ 0x030F, 0xD8 },
>>  	{ 0x0310, 0x00 },
>> +
>> +	{ 0x0114, 0x03 },
>> +	{ 0x0820, 0x0A },
>> +	{ 0x0821, 0x00 },
>> +	{ 0x0822, 0x00 },
>> +	{ 0x0823, 0x00 },
>> +};
>> +
>> +static const struct imx258_reg mipi_642mbps_24mhz_2l[] = {
>> +	{ 0x0136, 0x18 },
>> +	{ 0x0137, 0x00 },
>> +	{ 0x0301, 0x0A },
>> +	{ 0x0303, 0x02 },
>> +	{ 0x0305, 0x04 },
>> +	{ 0x0306, 0x00 },
>> +	{ 0x0307, 0x6B },
>> +	{ 0x0309, 0x0A },
>> +	{ 0x030B, 0x01 },
>> +	{ 0x030D, 0x02 },
>> +	{ 0x030E, 0x00 },
>> +	{ 0x030F, 0xD8 },
>> +	{ 0x0310, 0x00 },
>> +
>> +	{ 0x0114, 0x01 },
>>  	{ 0x0820, 0x0A },
>>  	{ 0x0821, 0x00 },
>>  	{ 0x0822, 0x00 },
>>  	{ 0x0823, 0x00 },
>>  };
>>  
>> -static const struct imx258_reg mipi_642mbps_24mhz[] = {
>> +static const struct imx258_reg mipi_642mbps_24mhz_4l[] = {
>>  	{ 0x0136, 0x18 },
>>  	{ 0x0137, 0x00 },
>>  	{ 0x0301, 0x05 },
>> @@ -186,6 +290,8 @@ static const struct imx258_reg mipi_642mbps_24mhz[] = {
>>  	{ 0x030E, 0x00 },
>>  	{ 0x030F, 0xD8 },
>>  	{ 0x0310, 0x00 },
>> +
>> +	{ 0x0114, 0x03 },
>>  	{ 0x0820, 0x0A },
>>  	{ 0x0821, 0x00 },
>>  	{ 0x0822, 0x00 },
>> @@ -240,7 +346,6 @@ static const struct imx258_reg mode_common_regs[] = {
>>  	{ 0x5F05, 0xED },
>>  	{ 0x0112, 0x0A },
>>  	{ 0x0113, 0x0A },
>> -	{ 0x0114, 0x03 },
>>  	{ 0x0342, 0x14 },
>>  	{ 0x0343, 0xE8 },
>>  	{ 0x0344, 0x00 },
>> @@ -359,11 +464,13 @@ enum {
>>  
>>  /*
>>   * pixel_rate = link_freq * data-rate * nr_of_lanes / bits_per_sample
>> - * data rate => double data rate; number of lanes => 4; bits per pixel => 10
>> + * data rate => double data rate;
>> + * number of lanes => (configurable 2 or 4);
>> + * bits per pixel => 10
>>   */
>> -static u64 link_freq_to_pixel_rate(u64 f)
>> +static u64 link_freq_to_pixel_rate(u64 f, unsigned int nlanes)
>>  {
>> -	f *= 2 * 4;
>> +	f *= 2 * nlanes;
>>  	do_div(f, 10);
>>  
>>  	return f;
>> @@ -393,15 +500,27 @@ static const struct imx258_link_freq_config link_freq_configs_19_2[] = {
>>  	[IMX258_LINK_FREQ_1267MBPS] = {
>>  		.pixels_per_line = IMX258_PPL_DEFAULT,
>>  		.reg_list = {
>> -			.num_of_regs = ARRAY_SIZE(mipi_1267mbps_19_2mhz),
>> -			.regs = mipi_1267mbps_19_2mhz,
>> +			[IMX258_2_LANE_MODE] = {
>> +				.num_of_regs = ARRAY_SIZE(mipi_1267mbps_19_2mhz_2l),
>> +				.regs = mipi_1267mbps_19_2mhz_2l,
>> +			},
>> +			[IMX258_4_LANE_MODE] = {
>> +				.num_of_regs = ARRAY_SIZE(mipi_1267mbps_19_2mhz_4l),
>> +				.regs = mipi_1267mbps_19_2mhz_4l,
>> +			},
>>  		}
>>  	},
>>  	[IMX258_LINK_FREQ_640MBPS] = {
>>  		.pixels_per_line = IMX258_PPL_DEFAULT,
>>  		.reg_list = {
>> -			.num_of_regs = ARRAY_SIZE(mipi_640mbps_19_2mhz),
>> -			.regs = mipi_640mbps_19_2mhz,
>> +			[IMX258_2_LANE_MODE] = {
>> +				.num_of_regs = ARRAY_SIZE(mipi_640mbps_19_2mhz_2l),
>> +				.regs = mipi_640mbps_19_2mhz_2l,
>> +			},
>> +			[IMX258_4_LANE_MODE] = {
>> +				.num_of_regs = ARRAY_SIZE(mipi_640mbps_19_2mhz_4l),
>> +				.regs = mipi_640mbps_19_2mhz_4l,
>> +			},
>>  		}
>>  	},
>>  };
>> @@ -410,15 +529,27 @@ static const struct imx258_link_freq_config link_freq_configs_24[] = {
>>  	[IMX258_LINK_FREQ_1267MBPS] = {
>>  		.pixels_per_line = IMX258_PPL_DEFAULT,
>>  		.reg_list = {
>> -			.num_of_regs = ARRAY_SIZE(mipi_1272mbps_24mhz),
>> -			.regs = mipi_1272mbps_24mhz,
>> +			[IMX258_2_LANE_MODE] = {
>> +				.num_of_regs = ARRAY_SIZE(mipi_1272mbps_24mhz_2l),
>> +				.regs = mipi_1272mbps_24mhz_2l,
>> +			},
>> +			[IMX258_4_LANE_MODE] = {
>> +				.num_of_regs = ARRAY_SIZE(mipi_1272mbps_24mhz_4l),
>> +				.regs = mipi_1272mbps_24mhz_4l,
>> +			},
>>  		}
>>  	},
>>  	[IMX258_LINK_FREQ_640MBPS] = {
>>  		.pixels_per_line = IMX258_PPL_DEFAULT,
>>  		.reg_list = {
>> -			.num_of_regs = ARRAY_SIZE(mipi_642mbps_24mhz),
>> -			.regs = mipi_642mbps_24mhz,
>> +			[IMX258_2_LANE_MODE] = {
>> +				.num_of_regs = ARRAY_SIZE(mipi_642mbps_24mhz_2l),
>> +				.regs = mipi_642mbps_24mhz_2l,
>> +			},
>> +			[IMX258_4_LANE_MODE] = {
>> +				.num_of_regs = ARRAY_SIZE(mipi_642mbps_24mhz_4l),
>> +				.regs = mipi_642mbps_24mhz_4l,
>> +			},
>>  		}
>>  	},
>>  };
>> @@ -477,6 +608,7 @@ struct imx258 {
>>  
>>  	const struct imx258_link_freq_config *link_freq_configs;
>>  	const s64 *link_freq_menu_items;
>> +	unsigned int nlanes;
>>  
>>  	/*
>>  	 * Mutex for serialized access:
>> @@ -782,7 +914,7 @@ static int imx258_set_pad_format(struct v4l2_subdev *sd,
>>  		__v4l2_ctrl_s_ctrl(imx258->link_freq, mode->link_freq_index);
>>  
>>  		link_freq = imx258->link_freq_menu_items[mode->link_freq_index];
>> -		pixel_rate = link_freq_to_pixel_rate(link_freq);
>> +		pixel_rate = link_freq_to_pixel_rate(link_freq, imx258->nlanes);
>>  		__v4l2_ctrl_s_ctrl_int64(imx258->pixel_rate, pixel_rate);
>>  		/* Update limits and set FPS to default */
>>  		vblank_def = imx258->cur_mode->vts_def -
>> @@ -811,11 +943,13 @@ static int imx258_start_streaming(struct imx258 *imx258)
>>  {
>>  	struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
>>  	const struct imx258_reg_list *reg_list;
>> +	const struct imx258_link_freq_config *link_freq_cfg;
>>  	int ret, link_freq_index;
>>  
>>  	/* Setup PLL */
>>  	link_freq_index = imx258->cur_mode->link_freq_index;
>> -	reg_list = &imx258->link_freq_configs[link_freq_index].reg_list;
>> +	link_freq_cfg = &imx258->link_freq_configs[link_freq_index];
>> +	reg_list = &link_freq_cfg->reg_list[imx258->nlanes == 2 ? 0 : 1];
>>  	ret = imx258_write_regs(imx258, reg_list->regs, reg_list->num_of_regs);
>>  	if (ret) {
>>  		dev_err(&client->dev, "%s failed to set plls\n", __func__);
>> @@ -1033,9 +1167,11 @@ static int imx258_init_controls(struct imx258 *imx258)
>>  		vflip->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>>  
>>  	pixel_rate_max =
>> -		link_freq_to_pixel_rate(imx258->link_freq_menu_items[0]);
>> +		link_freq_to_pixel_rate(imx258->link_freq_menu_items[0],
>> +					imx258->nlanes);
>>  	pixel_rate_min =
>> -		link_freq_to_pixel_rate(imx258->link_freq_menu_items[1]);
>> +		link_freq_to_pixel_rate(imx258->link_freq_menu_items[1],
>> +					imx258->nlanes);
>>  	/* By default, PIXEL_RATE is read only */
>>  	imx258->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx258_ctrl_ops,
>>  				V4L2_CID_PIXEL_RATE,
>> @@ -1132,6 +1268,10 @@ static int imx258_get_regulators(struct imx258 *imx258,
>>  static int imx258_probe(struct i2c_client *client)
>>  {
>>  	struct imx258 *imx258;
>> +	struct fwnode_handle *endpoint;
>> +	struct v4l2_fwnode_endpoint ep = {
>> +		.bus_type = V4L2_MBUS_CSI2_DPHY
>> +	};
>>  	int ret;
>>  	u32 val = 0;
>>  
>> @@ -1172,13 +1312,35 @@ static int imx258_probe(struct i2c_client *client)
>>  		return -EINVAL;
>>  	}
>>  
>> +	endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), NULL);
>> +	if (!endpoint) {
>> +		dev_err(&client->dev, "Endpoint node not found\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep);
> 
> Here you're obtaining the list of supported link frequencies from the
> firmware but it is not validated (nor it was validated by the driver
> previously). I'd regard that a driver bug but fixing it at this point could
> introduce adverse effects elsewhere.
> 
> I think what I'd do here is that I'd ignore the issue if there are no
> frequencies defined for the endpoint but if there are, then enable only
> those that are listed in the endpoint.
> 
> Could you add a patch to do this, please? v4l2_link_freq_to_bitmap() has
> been recently added to facilitate this.
> 

I can give this a try, it would be similar to this patch that you submitted
earlier for the imx319 here
https://github.com/torvalds/linux/commit/726a09c1b6890887b7388745a26c8e93867780ca
right? 

>> +	fwnode_handle_put(endpoint);
>> +	if (ret) {
>> +		dev_err(&client->dev, "Parsing endpoint node failed\n");
>> +		return ret;
>> +	}
>> +
>> +	/* Get number of data lanes */
>> +	imx258->nlanes = ep.bus.mipi_csi2.num_data_lanes;
>> +	if (imx258->nlanes != 2 && imx258->nlanes != 4) {
>> +		dev_err(&client->dev, "Invalid data lanes: %u\n",
>> +			imx258->nlanes);
>> +		ret = -EINVAL;
>> +		goto error_endpoint_free;
>> +	}
>> +
>>  	/* Initialize subdev */
>>  	v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
>>  
>>  	/* Will be powered off via pm_runtime_idle */
>>  	ret = imx258_power_on(&client->dev);
>>  	if (ret)
>> -		return ret;
>> +		goto error_endpoint_free;
>>  
>>  	/* Check module identity */
>>  	ret = imx258_identify_module(imx258);
>> @@ -1211,6 +1373,7 @@ static int imx258_probe(struct i2c_client *client)
>>  	pm_runtime_set_active(&client->dev);
>>  	pm_runtime_enable(&client->dev);
>>  	pm_runtime_idle(&client->dev);
>> +	v4l2_fwnode_endpoint_free(&ep);
>>  
>>  	return 0;
>>  
>> @@ -1223,6 +1386,9 @@ static int imx258_probe(struct i2c_client *client)
>>  error_identify:
>>  	imx258_power_off(&client->dev);
>>  
>> +error_endpoint_free:
>> +	v4l2_fwnode_endpoint_free(&ep);
>> +
>>  	return ret;
>>  }
>>  
> 


^ permalink raw reply

* Re: [PATCH v5 1/7] iio: accel: adxl345: Make data_range obsolete
From: Lothar Rubusch @ 2024-03-29  0:03 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: lars, Michael.Hennerich, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, linux-iio, devicetree, linux-kernel, eraretuya
In-Reply-To: <20240328133720.7dfd46b0@jic23-huawei>

On Thu, Mar 28, 2024 at 2:37 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Wed, 27 Mar 2024 22:03:14 +0000
> Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> > Replace write() data_format by regmap_update_bits(), because bus specific
> > pre-configuration may have happened before on the same register. For
> > further updates to the data_format register then bus pre-configuration
> > needs to be masked out.
> >
> > Remove the data_range field from the struct adxl345_data, because it is
> > not used anymore.
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> > ---
> >  drivers/iio/accel/adxl345_core.c | 18 ++++++++++++------
> >  1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> > index 8bd30a23e..35df5e372 100644
> > --- a/drivers/iio/accel/adxl345_core.c
> > +++ b/drivers/iio/accel/adxl345_core.c
> > @@ -37,7 +37,15 @@
> >  #define ADXL345_POWER_CTL_MEASURE    BIT(3)
> >  #define ADXL345_POWER_CTL_STANDBY    0x00
> >
> > +#define ADXL345_DATA_FORMAT_RANGE    GENMASK(1, 0) /* Set the g range */
> > +#define ADXL345_DATA_FORMAT_JUSTIFY  BIT(2) /* Left-justified (MSB) mode */
> >  #define ADXL345_DATA_FORMAT_FULL_RES BIT(3) /* Up to 13-bits resolution */
> > +#define ADXL345_DATA_FORMAT_SELF_TEST        BIT(7) /* Enable a self test */
> > +#define ADXL345_DATA_FORMAT_MSK              (ADXL345_DATA_FORMAT_RANGE | \
> > +                                      ADXL345_DATA_FORMAT_JUSTIFY |  \
> > +                                      ADXL345_DATA_FORMAT_FULL_RES | \
> > +                                      ADXL345_DATA_FORMAT_SELF_TEST)
> This needs renaming.  It's not a mask of everything in the register, or
> even just of everything related to format.
>
> Actually I'd just not have this definition.  Use the build up value
> from all the submasks at the call site.  Then we are just making it clear
> only a subset of fields are being cleared.
>
I understand this solution was not very useful. I'm not sure, I
understood you correctly. Please have a look into v6 if this matches
your comment.
Now, I remove the mask, instead I use a local variable in core's
probe() for the update mask. I added a comment. Nevertheless, I keep
the used flags for FORMAT_DATA. Does this go into the direction of
using the build up value from the submasks at the call site?

> Jonathan
>
> > +
> >  #define ADXL345_DATA_FORMAT_2G               0
> >  #define ADXL345_DATA_FORMAT_4G               1
> >  #define ADXL345_DATA_FORMAT_8G               2
> > @@ -48,7 +56,6 @@
> >  struct adxl345_data {
> >       const struct adxl345_chip_info *info;
> >       struct regmap *regmap;
> > -     u8 data_range;
> >  };
> >
> >  #define ADXL345_CHANNEL(index, axis) {                                       \
> > @@ -218,15 +225,14 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap)
> >
> >       data = iio_priv(indio_dev);
> >       data->regmap = regmap;
> > -     /* Enable full-resolution mode */
> > -     data->data_range = ADXL345_DATA_FORMAT_FULL_RES;
> >       data->info = device_get_match_data(dev);
> >       if (!data->info)
> >               return -ENODEV;
> >
> > -     ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT,
> > -                        data->data_range);
> > -     if (ret < 0)
> > +     /* Enable full-resolution mode */
> > +     ret = regmap_update_bits(regmap, ADXL345_REG_DATA_FORMAT,
> > +                              ADXL345_DATA_FORMAT_MSK, ADXL345_DATA_FORMAT_FULL_RES);
> > +     if (ret)
> >               return dev_err_probe(dev, ret, "Failed to set data range\n");
> >
> >       indio_dev->name = data->info->name;
>

^ permalink raw reply

* Re: [PATCH v5 7/7] iio: accel: adxl345: Add spi-3wire option
From: Lothar Rubusch @ 2024-03-29  0:33 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: lars, Michael.Hennerich, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, linux-iio, devicetree, linux-kernel, eraretuya
In-Reply-To: <20240328133927.7e49f3bf@jic23-huawei>

On Thu, Mar 28, 2024 at 2:39 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Wed, 27 Mar 2024 22:03:20 +0000
> Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> > Add a setup function implementation to the spi module to enable spi-3wire
> > as option when specified in the device-tree.
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> > ---
> >  drivers/iio/accel/adxl345.h     |  2 ++
> >  drivers/iio/accel/adxl345_spi.c | 12 +++++++++++-
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
> > index 4ea9341d4..e6bc3591c 100644
> > --- a/drivers/iio/accel/adxl345.h
> > +++ b/drivers/iio/accel/adxl345.h
> > @@ -30,6 +30,8 @@
> >  #define ADXL345_POWER_CTL_MEASURE    BIT(3)
> >  #define ADXL345_POWER_CTL_STANDBY    0x00
> >
> > +#define ADXL345_DATA_FORMAT_SPI_3WIRE        BIT(6) /* 3-wire SPI mode */
> > +
> >  #define ADXL345_DATA_FORMAT_RANGE    GENMASK(1, 0) /* Set the g range */
> >  #define ADXL345_DATA_FORMAT_JUSTIFY  BIT(2) /* Left-justified (MSB) mode */
> >  #define ADXL345_DATA_FORMAT_FULL_RES BIT(3) /* Up to 13-bits resolution */
> > diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
> > index 1c0513bd3..f145d5c1d 100644
> > --- a/drivers/iio/accel/adxl345_spi.c
> > +++ b/drivers/iio/accel/adxl345_spi.c
> > @@ -20,6 +20,16 @@ static const struct regmap_config adxl345_spi_regmap_config = {
> >       .read_flag_mask = BIT(7) | BIT(6),
> >  };
> >
> > +static int adxl345_spi_setup(struct device *dev, struct regmap *regmap)
> > +{
> > +     struct spi_device *spi = container_of(dev, struct spi_device, dev);
> > +
> > +     if (spi->mode & SPI_3WIRE)
> > +             return regmap_write(regmap, ADXL345_REG_DATA_FORMAT,
> > +                                 ADXL345_DATA_FORMAT_SPI_3WIRE);
> Your earlier patch carefully (I think) left one or two fields alone, then
> this write just comes in and changes them. In particular INT_INVERT.
>
Why do you refer here to INT_INVERT? In this code above I try to set
SPI to 3 lines. Since this is a SPI configuration, i.e. bus specific,
it happens in adxl345_spi.c. Passing this function to the bus
independent adxl345_core.c file allows to configure the bus first.
Therefore, I'm using the update function in core masking out the SPI
filag.

My reason why I try to keep INT_INVERT out is different. There is
another driver for the same part in the kernel:
./drivers/input/misc/adxl34x.c - This is a input driver, using the
interrupts of the adxl345 for the input device implementation. I
assumed, that in the iio implementation there won't be interrupt
handling for the adx345, since it is not an input device. Does this
make sense?

> If it doesn't makes sense to write it there, either write that bit
> every time here, or leave it alone every time.  Not decide on whether
> to write the bit based on SPI_3WIRE or not.  As far as I know they
> are unconnected features.
>
I think I did not understand. Could you please specify a bit more?
When spi-3wire is configured in the DT it has to be parsed and handled
in the bus specific part, i.e. the adxl345_spi.c Therefore I configure
SPI_3WIRE there. I don't want to place SPI specific code in the core
file.

> > +     return 0;
> > +}
> > +
> >  static int adxl345_spi_probe(struct spi_device *spi)
> >  {
> >       struct regmap *regmap;
> > @@ -33,7 +43,7 @@ static int adxl345_spi_probe(struct spi_device *spi)
> >       if (IS_ERR(regmap))
> >               return dev_err_probe(&spi->dev, PTR_ERR(regmap), "Error initializing regmap\n");
> >
> > -     return adxl345_core_probe(&spi->dev, regmap, NULL);
> > +     return adxl345_core_probe(&spi->dev, regmap, adxl345_spi_setup);
> >  }
> >
> >  static const struct adxl345_chip_info adxl345_spi_info = {
>

^ permalink raw reply

* RE: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG bindings
From: Klymenko, Anatoliy @ 2024-03-29  0:38 UTC (permalink / raw)
  To: Krzysztof Kozlowski, 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
  Cc: 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: <c0d70ba9-34ef-4121-834d-4d107f03d7f0@linaro.org>

Hi Krzysztof,

Thank you for the feedback.

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Saturday, March 23, 2024 3:21 AM
> To: Klymenko, Anatoliy <Anatoliy.Klymenko@amd.com>; Laurent Pinchart
> <laurent.pinchart@ideasonboard.com>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>; Maxime Ripard
> <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>;
> David Airlie <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>; Simek,
> Michal <michal.simek@amd.com>; Andrzej Hajda
> <andrzej.hajda@intel.com>; Neil Armstrong <neil.armstrong@linaro.org>;
> Robert Foss <rfoss@kernel.org>; Jonas Karlman <jonas@kwiboo.se>; Jernej
> Skrabec <jernej.skrabec@gmail.com>; Rob Herring <robh+dt@kernel.org>;
> Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>; 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
> Subject: Re: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG bindings
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> On 22/03/2024 20:12, Klymenko, Anatoliy wrote:
> > Hi Krzysztof,
> >
> > Thanks a lot for the review.
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> Sent: Thursday, March 21, 2024 10:59 PM
> >> To: Klymenko, Anatoliy <Anatoliy.Klymenko@amd.com>; Laurent Pinchart
> >> <laurent.pinchart@ideasonboard.com>; Maarten Lankhorst
> >> <maarten.lankhorst@linux.intel.com>; Maxime Ripard
> >> <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>;
> David
> >> Airlie <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>; Simek,
> >> Michal <michal.simek@amd.com>; Andrzej Hajda
> >> <andrzej.hajda@intel.com>; Neil Armstrong
> >> <neil.armstrong@linaro.org>; Robert Foss <rfoss@kernel.org>; Jonas
> >> Karlman <jonas@kwiboo.se>; Jernej Skrabec
> <jernej.skrabec@gmail.com>;
> >> Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> >> <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> >> <conor+dt@kernel.org>; Mauro Carvalho Chehab <mchehab@kernel.org>
> >> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>; 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
> >> Subject: Re: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG
> >> bindings
> >>
> >> Caution: This message originated from an External Source. Use proper
> >> caution when opening attachments, clicking links, or responding.
> >>
> >>
> >> On 21/03/2024 21:43, Anatoliy Klymenko wrote:
> >>> diff --git a/include/dt-bindings/media/media-bus-format.h
> >>> b/include/dt-
> >> bindings/media/media-bus-format.h
> >>> new file mode 100644
> >>> index 000000000000..60fc6e11dabc
> >>> --- /dev/null
> >>> +++ b/include/dt-bindings/media/media-bus-format.h
> >>> @@ -0,0 +1,177 @@
> >>> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> >>> +/*
> >>> + * Media Bus API header
> >>> + *
> >>> + * Copyright (C) 2009, Guennadi Liakhovetski
> >>> +<g.liakhovetski@gmx.de>
> >>> + *
> >>> + * This program is free software; you can redistribute it and/or
> >>> +modify
> >>> + * it under the terms of the GNU General Public License version 2
> >>> +as
> >>> + * published by the Free Software Foundation.
> >>
> >> That's not true. Your SPDX tells something entirely different.
> >>
> >
> > Thank you - I'll see how to fix it.
> >
> >> Anyway, you did not explain why you need to copy anything anywhere.
> >>
> >> Specifically, random hex values *are not bindings*.
> >>
> >
> > The same media bus format values are being used by the reference
> > driver in patch #9. And, as far as I know, we cannot use headers from
> > Linux API headers directly (at least I
> 
> I don't understand what does it mean. You can use in your driver whatever
> headers you wish, I don't care about them.
> 
> 
> noticed the same pattern in ../dt-bindings/sdtv-standarts.h for instance).
> What would be the best approach to reusing the same defines on DT and
> driver sides from your point of view? Symlink maybe?
> >
> 
> Wrap your messages to match mailing list discussion style. There are no
> defines used in DT. If there are, show me them in *THIS* or other
> *upstreamed* (being upstreamed) patchset.
> 

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
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?

> Whatever you have out of tree or "DO NOT MERGE" does not matter and
> does not justify anything.
> 
> 
> Best regards,
> Krzysztof

Thank you,
Anatoliy


^ permalink raw reply

* [PATCH v6 0/7]  iio: accel: adxl345: Add spi-3wire feature
From: Lothar Rubusch @ 2024-03-29  0:40 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch

Pass a function setup() as pointer from SPI/I2C specific modules to the
core module. Implement setup() to pass the spi-3wire bus option, if
declared in the device-tree.

In the core module, then update data_format register configuration bits
instead of overwriting it. The changes allow to remove a data_range field.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
V1 -> V2: Split into spi-3wire and refactoring
V2 -> V3: Split further, focus on needed changesets
V3 -> V4: Drop "Remove single info instances";
          split "Group bus configuration" into separat
          comment patch; reorder patch set
V4 -> V5: Refrase comments; Align comments to 75; rebuild FORMAT_MASK by
          available flags; fix indention
V5 -> V6: Remove FORMAT_MASK by a local variable on call site;
          Refrase comments;
          Remove unneeded include


Lothar Rubusch (7):
  iio: accel: adxl345: Make data_range obsolete
  iio: accel: adxl345: Group bus configuration
  iio: accel: adxl345: Move defines to header
  dt-bindings: iio: accel: adxl345: Add spi-3wire
  iio: accel: adxl345: Pass function pointer to core
  iio: accel: adxl345: Add comment to probe
  iio: accel: adxl345: Add spi-3wire option

 .../bindings/iio/accel/adi,adxl345.yaml       |  2 +
 drivers/iio/accel/adxl345.h                   | 36 +++++++++-
 drivers/iio/accel/adxl345_core.c              | 72 +++++++++----------
 drivers/iio/accel/adxl345_i2c.c               |  2 +-
 drivers/iio/accel/adxl345_spi.c               | 12 +++-
 5 files changed, 84 insertions(+), 40 deletions(-)

-- 
2.25.1


^ permalink raw reply

* [PATCH v6 1/7] iio: accel: adxl345: Make data_range obsolete
From: Lothar Rubusch @ 2024-03-29  0:40 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch
In-Reply-To: <20240329004030.16153-1-l.rubusch@gmail.com>

Replace write() data_format by regmap_update_bits() to keep bus specific
pre-configuration which might have happened before on the same register.
The bus specific bits in data_format register then need to be masked out,

Remove the data_range field from the struct adxl345_data, because it is
not used anymore.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 8bd30a23e..f4dec5ff1 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -37,7 +37,11 @@
 #define ADXL345_POWER_CTL_MEASURE	BIT(3)
 #define ADXL345_POWER_CTL_STANDBY	0x00
 
-#define ADXL345_DATA_FORMAT_FULL_RES	BIT(3) /* Up to 13-bits resolution */
+#define ADXL345_DATA_FORMAT_RANGE	GENMASK(1, 0)	/* Set the g range */
+#define ADXL345_DATA_FORMAT_JUSTIFY	BIT(2)	/* Left-justified (MSB) mode */
+#define ADXL345_DATA_FORMAT_FULL_RES	BIT(3)	/* Up to 13-bits resolution */
+#define ADXL345_DATA_FORMAT_SELF_TEST	BIT(7)	/* Enable a self test */
+
 #define ADXL345_DATA_FORMAT_2G		0
 #define ADXL345_DATA_FORMAT_4G		1
 #define ADXL345_DATA_FORMAT_8G		2
@@ -48,7 +52,6 @@
 struct adxl345_data {
 	const struct adxl345_chip_info *info;
 	struct regmap *regmap;
-	u8 data_range;
 };
 
 #define ADXL345_CHANNEL(index, axis) {					\
@@ -202,6 +205,7 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap)
 	struct adxl345_data *data;
 	struct iio_dev *indio_dev;
 	u32 regval;
+	unsigned int data_format_mask;
 	int ret;
 
 	ret = regmap_read(regmap, ADXL345_REG_DEVID, &regval);
@@ -218,15 +222,23 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap)
 
 	data = iio_priv(indio_dev);
 	data->regmap = regmap;
-	/* Enable full-resolution mode */
-	data->data_range = ADXL345_DATA_FORMAT_FULL_RES;
 	data->info = device_get_match_data(dev);
 	if (!data->info)
 		return -ENODEV;
 
-	ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT,
-			   data->data_range);
-	if (ret < 0)
+	/*
+	 * Only allow updates of bus independent bits to the data_format
+	 * register. Keep the bus specific pre-configuration.
+	 */
+	data_format_mask = (ADXL345_DATA_FORMAT_RANGE |
+				  ADXL345_DATA_FORMAT_JUSTIFY |
+				  ADXL345_DATA_FORMAT_FULL_RES |
+				  ADXL345_DATA_FORMAT_SELF_TEST);
+
+	/* Enable full-resolution mode */
+	ret = regmap_update_bits(regmap, ADXL345_REG_DATA_FORMAT,
+			data_format_mask, ADXL345_DATA_FORMAT_FULL_RES);
+	if (ret)
 		return dev_err_probe(dev, ret, "Failed to set data range\n");
 
 	indio_dev->name = data->info->name;
-- 
2.25.1


^ permalink raw reply related

* [PATCH v6 2/7] iio: accel: adxl345: Group bus configuration
From: Lothar Rubusch @ 2024-03-29  0:40 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch
In-Reply-To: <20240329004030.16153-1-l.rubusch@gmail.com>

Group the indio_dev initialization and bus configuration for improved
readability.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index f4dec5ff1..78ac6ea54 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -226,6 +226,12 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap)
 	if (!data->info)
 		return -ENODEV;
 
+	indio_dev->name = data->info->name;
+	indio_dev->info = &adxl345_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = adxl345_channels;
+	indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
+
 	/*
 	 * Only allow updates of bus independent bits to the data_format
 	 * register. Keep the bus specific pre-configuration.
@@ -241,12 +247,6 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to set data range\n");
 
-	indio_dev->name = data->info->name;
-	indio_dev->info = &adxl345_info;
-	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = adxl345_channels;
-	indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
-
 	/* Enable measurement mode */
 	ret = adxl345_powerup(data->regmap);
 	if (ret < 0)
-- 
2.25.1


^ permalink raw reply related

* [PATCH v6 3/7] iio: accel: adxl345: Move defines to header
From: Lothar Rubusch @ 2024-03-29  0:40 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch
In-Reply-To: <20240329004030.16153-1-l.rubusch@gmail.com>

Move defines from core to the header file. Keep the defines block together
in one location.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345.h      | 32 ++++++++++++++++++++++++++++++++
 drivers/iio/accel/adxl345_core.c | 32 --------------------------------
 2 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
index 284bd387c..732820d34 100644
--- a/drivers/iio/accel/adxl345.h
+++ b/drivers/iio/accel/adxl345.h
@@ -8,6 +8,38 @@
 #ifndef _ADXL345_H_
 #define _ADXL345_H_
 
+#define ADXL345_REG_DEVID		0x00
+#define ADXL345_REG_OFSX		0x1E
+#define ADXL345_REG_OFSY		0x1F
+#define ADXL345_REG_OFSZ		0x20
+#define ADXL345_REG_OFS_AXIS(index)	(ADXL345_REG_OFSX + (index))
+#define ADXL345_REG_BW_RATE		0x2C
+#define ADXL345_REG_POWER_CTL		0x2D
+#define ADXL345_REG_DATA_FORMAT		0x31
+#define ADXL345_REG_DATAX0		0x32
+#define ADXL345_REG_DATAY0		0x34
+#define ADXL345_REG_DATAZ0		0x36
+#define ADXL345_REG_DATA_AXIS(index)	\
+	(ADXL345_REG_DATAX0 + (index) * sizeof(__le16))
+
+#define ADXL345_BW_RATE			GENMASK(3, 0)
+#define ADXL345_BASE_RATE_NANO_HZ	97656250LL
+
+#define ADXL345_POWER_CTL_MEASURE	BIT(3)
+#define ADXL345_POWER_CTL_STANDBY	0x00
+
+#define ADXL345_DATA_FORMAT_RANGE	GENMASK(1, 0)	/* Set the g range */
+#define ADXL345_DATA_FORMAT_JUSTIFY	BIT(2)	/* Left-justified (MSB) mode */
+#define ADXL345_DATA_FORMAT_FULL_RES	BIT(3)	/* Up to 13-bits resolution */
+#define ADXL345_DATA_FORMAT_SELF_TEST	BIT(7)	/* Enable a self test */
+
+#define ADXL345_DATA_FORMAT_2G		0
+#define ADXL345_DATA_FORMAT_4G		1
+#define ADXL345_DATA_FORMAT_8G		2
+#define ADXL345_DATA_FORMAT_16G		3
+
+#define ADXL345_DEVID			0xE5
+
 /*
  * In full-resolution mode, scale factor is maintained at ~4 mg/LSB
  * in all g ranges.
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 78ac6ea54..2d229fa80 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -17,38 +17,6 @@
 
 #include "adxl345.h"
 
-#define ADXL345_REG_DEVID		0x00
-#define ADXL345_REG_OFSX		0x1e
-#define ADXL345_REG_OFSY		0x1f
-#define ADXL345_REG_OFSZ		0x20
-#define ADXL345_REG_OFS_AXIS(index)	(ADXL345_REG_OFSX + (index))
-#define ADXL345_REG_BW_RATE		0x2C
-#define ADXL345_REG_POWER_CTL		0x2D
-#define ADXL345_REG_DATA_FORMAT		0x31
-#define ADXL345_REG_DATAX0		0x32
-#define ADXL345_REG_DATAY0		0x34
-#define ADXL345_REG_DATAZ0		0x36
-#define ADXL345_REG_DATA_AXIS(index)	\
-	(ADXL345_REG_DATAX0 + (index) * sizeof(__le16))
-
-#define ADXL345_BW_RATE			GENMASK(3, 0)
-#define ADXL345_BASE_RATE_NANO_HZ	97656250LL
-
-#define ADXL345_POWER_CTL_MEASURE	BIT(3)
-#define ADXL345_POWER_CTL_STANDBY	0x00
-
-#define ADXL345_DATA_FORMAT_RANGE	GENMASK(1, 0)	/* Set the g range */
-#define ADXL345_DATA_FORMAT_JUSTIFY	BIT(2)	/* Left-justified (MSB) mode */
-#define ADXL345_DATA_FORMAT_FULL_RES	BIT(3)	/* Up to 13-bits resolution */
-#define ADXL345_DATA_FORMAT_SELF_TEST	BIT(7)	/* Enable a self test */
-
-#define ADXL345_DATA_FORMAT_2G		0
-#define ADXL345_DATA_FORMAT_4G		1
-#define ADXL345_DATA_FORMAT_8G		2
-#define ADXL345_DATA_FORMAT_16G		3
-
-#define ADXL345_DEVID			0xE5
-
 struct adxl345_data {
 	const struct adxl345_chip_info *info;
 	struct regmap *regmap;
-- 
2.25.1


^ permalink raw reply related

* [PATCH v6 4/7] dt-bindings: iio: accel: adxl345: Add spi-3wire
From: Lothar Rubusch @ 2024-03-29  0:40 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch,
	Krzysztof Kozlowski
In-Reply-To: <20240329004030.16153-1-l.rubusch@gmail.com>

Add spi-3wire because the device allows to be configured for spi 3-wire
communication.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml
index 07cacc3f6..280ed479e 100644
--- a/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml
@@ -32,6 +32,8 @@ properties:
 
   spi-cpol: true
 
+  spi-3wire: true
+
   interrupts:
     maxItems: 1
 
-- 
2.25.1


^ permalink raw reply related

* [PATCH v6 5/7] iio: accel: adxl345: Pass function pointer to core
From: Lothar Rubusch @ 2024-03-29  0:40 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch
In-Reply-To: <20240329004030.16153-1-l.rubusch@gmail.com>

Provide a way for bus specific pre-configuration by adding a function
pointer argument to the driver core's probe() function, and keep
the driver core implementation bus independent.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345.h      |  3 ++-
 drivers/iio/accel/adxl345_core.c | 10 +++++++++-
 drivers/iio/accel/adxl345_i2c.c  |  2 +-
 drivers/iio/accel/adxl345_spi.c  |  2 +-
 4 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
index 732820d34..e859c01d4 100644
--- a/drivers/iio/accel/adxl345.h
+++ b/drivers/iio/accel/adxl345.h
@@ -60,6 +60,7 @@ struct adxl345_chip_info {
 	int uscale;
 };
 
-int adxl345_core_probe(struct device *dev, struct regmap *regmap);
+int adxl345_core_probe(struct device *dev, struct regmap *regmap,
+		       int (*setup)(struct device*, struct regmap*));
 
 #endif /* _ADXL345_H_ */
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 2d229fa80..83d7e7d4e 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -168,7 +168,8 @@ static void adxl345_powerdown(void *regmap)
 	regmap_write(regmap, ADXL345_REG_POWER_CTL, ADXL345_POWER_CTL_STANDBY);
 }
 
-int adxl345_core_probe(struct device *dev, struct regmap *regmap)
+int adxl345_core_probe(struct device *dev, struct regmap *regmap,
+		       int (*setup)(struct device*, struct regmap*))
 {
 	struct adxl345_data *data;
 	struct iio_dev *indio_dev;
@@ -176,6 +177,13 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap)
 	unsigned int data_format_mask;
 	int ret;
 
+	/* Perform optional initial bus specific configuration */
+	if (setup) {
+		ret = setup(dev, regmap);
+		if (ret)
+			return ret;
+	}
+
 	ret = regmap_read(regmap, ADXL345_REG_DEVID, &regval);
 	if (ret < 0)
 		return dev_err_probe(dev, ret, "Error reading device ID\n");
diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c
index a3084b0a8..4065b8f7c 100644
--- a/drivers/iio/accel/adxl345_i2c.c
+++ b/drivers/iio/accel/adxl345_i2c.c
@@ -27,7 +27,7 @@ static int adxl345_i2c_probe(struct i2c_client *client)
 	if (IS_ERR(regmap))
 		return dev_err_probe(&client->dev, PTR_ERR(regmap), "Error initializing regmap\n");
 
-	return adxl345_core_probe(&client->dev, regmap);
+	return adxl345_core_probe(&client->dev, regmap, NULL);
 }
 
 static const struct adxl345_chip_info adxl345_i2c_info = {
diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
index 93ca349f1..1c0513bd3 100644
--- a/drivers/iio/accel/adxl345_spi.c
+++ b/drivers/iio/accel/adxl345_spi.c
@@ -33,7 +33,7 @@ static int adxl345_spi_probe(struct spi_device *spi)
 	if (IS_ERR(regmap))
 		return dev_err_probe(&spi->dev, PTR_ERR(regmap), "Error initializing regmap\n");
 
-	return adxl345_core_probe(&spi->dev, regmap);
+	return adxl345_core_probe(&spi->dev, regmap, NULL);
 }
 
 static const struct adxl345_chip_info adxl345_spi_info = {
-- 
2.25.1


^ permalink raw reply related

* [PATCH v6 6/7] iio: accel: adxl345: Add comment to probe
From: Lothar Rubusch @ 2024-03-29  0:40 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch
In-Reply-To: <20240329004030.16153-1-l.rubusch@gmail.com>

Add a comment to the probe() function to describe the passed function
pointer argument in particular.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 83d7e7d4e..511c27eb3 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -168,6 +168,16 @@ static void adxl345_powerdown(void *regmap)
 	regmap_write(regmap, ADXL345_REG_POWER_CTL, ADXL345_POWER_CTL_STANDBY);
 }
 
+/**
+ * adxl345_core_probe() - probe and setup for the adxl345 accelerometer,
+ *                        also covers the adlx375 accelerometer
+ * @dev:	Driver model representation of the device
+ * @regmap:	Regmap instance for the device
+ * @setup:	Setup routine to be executed right before the standard device
+ *		setup
+ *
+ * Return: 0 on success, negative errno on error
+ */
 int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 		       int (*setup)(struct device*, struct regmap*))
 {
-- 
2.25.1


^ permalink raw reply related

* [PATCH v6 7/7] iio: accel: adxl345: Add spi-3wire option
From: Lothar Rubusch @ 2024-03-29  0:40 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch
In-Reply-To: <20240329004030.16153-1-l.rubusch@gmail.com>

Add a setup function implementation to the spi module to enable spi-3wire
when specified in the device-tree.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345.h     |  1 +
 drivers/iio/accel/adxl345_spi.c | 12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
index e859c01d4..3d5c8719d 100644
--- a/drivers/iio/accel/adxl345.h
+++ b/drivers/iio/accel/adxl345.h
@@ -31,6 +31,7 @@
 #define ADXL345_DATA_FORMAT_RANGE	GENMASK(1, 0)	/* Set the g range */
 #define ADXL345_DATA_FORMAT_JUSTIFY	BIT(2)	/* Left-justified (MSB) mode */
 #define ADXL345_DATA_FORMAT_FULL_RES	BIT(3)	/* Up to 13-bits resolution */
+#define ADXL345_DATA_FORMAT_SPI_3WIRE	BIT(6)	/* 3-wire SPI mode */
 #define ADXL345_DATA_FORMAT_SELF_TEST	BIT(7)	/* Enable a self test */
 
 #define ADXL345_DATA_FORMAT_2G		0
diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
index 1c0513bd3..f145d5c1d 100644
--- a/drivers/iio/accel/adxl345_spi.c
+++ b/drivers/iio/accel/adxl345_spi.c
@@ -20,6 +20,16 @@ static const struct regmap_config adxl345_spi_regmap_config = {
 	.read_flag_mask = BIT(7) | BIT(6),
 };
 
+static int adxl345_spi_setup(struct device *dev, struct regmap *regmap)
+{
+	struct spi_device *spi = container_of(dev, struct spi_device, dev);
+
+	if (spi->mode & SPI_3WIRE)
+		return regmap_write(regmap, ADXL345_REG_DATA_FORMAT,
+				    ADXL345_DATA_FORMAT_SPI_3WIRE);
+	return 0;
+}
+
 static int adxl345_spi_probe(struct spi_device *spi)
 {
 	struct regmap *regmap;
@@ -33,7 +43,7 @@ static int adxl345_spi_probe(struct spi_device *spi)
 	if (IS_ERR(regmap))
 		return dev_err_probe(&spi->dev, PTR_ERR(regmap), "Error initializing regmap\n");
 
-	return adxl345_core_probe(&spi->dev, regmap, NULL);
+	return adxl345_core_probe(&spi->dev, regmap, adxl345_spi_setup);
 }
 
 static const struct adxl345_chip_info adxl345_spi_info = {
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH 1/2] V2 arm64: dts: qcom: Add support for Samsung Galaxy Z Fold5
From: Konrad Dybcio @ 2024-03-29  0:52 UTC (permalink / raw)
  To: serdeliuk, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20240329-v2-dts-add-support-for-samsung-galaxy-zfold5-v1-1-9a91e635cacc@yahoo.com>

On 29.03.2024 12:08 AM, Alexandru Marc Serdeliuc via B4 Relay wrote:
> From: Alexandru Marc Serdeliuc <serdeliuk@yahoo.com>
> 
> Add support for Samsung Galaxy Z Fold5 (q5q) foldable phone
> 
> Currently working features:
> - Framebuffer
> - UFS
> - i2c
> - Buttons
> 
> Signed-off-by: Alexandru Marc Serdeliuc <serdeliuk@yahoo.com>
> ---

Your commit title now includes "V2". Move it inside the square braces the
next time around, so it's like [PATCH v3 1/2]. With b4, this should be done
automagically, though..

[...]

> +/ {
> +	model = "Samsung Galaxy Z Fold5";
> +	compatible = "samsung,q5q", "qcom,sm8550";
> +	#address-cells = <0x02>;
> +	#size-cells = <0x02>;

These two can go

[...]

> +	reserved-memory {
> +		/*
> +		 * The bootloader will only keep display hardware enabled
> +		 * if this memory region is named exactly 'splash_region'
> +		 */

Ouch.

[...]

> +		vreg_l15b_1p8: ldo15 {
> +			regulator-name = "vreg_l15b_1p8";
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +			regulator-allow-set-load;
> +			regulator-allowed-modes = <RPMH_REGULATOR_MODE_LPM
> +						   RPMH_REGULATOR_MODE_HPM>;
> +			regulator-always-on;

Any particular reason as to why?

[...]

> +&remoteproc_adsp {
> +	firmware-name = "qcom/sm8550/adsp.mbn",
> +			"qcom/sm8550/adsp_dtb.mbn";
> +	status = "okay";
> +};
> +
> +&remoteproc_cdsp {
> +	firmware-name = "qcom/sm8550/cdsp.mbn",
> +			"qcom/sm8550/cdsp_dtb.mbn";
> +	status = "okay";
> +};
> +
> +&remoteproc_mpss {
> +	firmware-name = "qcom/sm8550/modem.mbn",
> +			"qcom/sm8550/modem_dtb.mbn";
> +	status = "okay";

Unless you stole one from the factory, these firmwares will not
load on your phone..

> +};
> +
> +&sleep_clk {
> +	clock-frequency = <32000>;
> +};
> +
> +&tlmm {
> +	gpio-reserved-ranges = <36 4>, <50 2>;

Would you have an idea what these GPIOs are used for?

Konrad

^ permalink raw reply

* Re: [PATCH 2/2] v2 arm64: dts: qcom: Add support for Samsung Galaxy Z Fold5
From: Konrad Dybcio @ 2024-03-29  0:54 UTC (permalink / raw)
  To: serdeliuk, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20240329-v2-dts-add-support-for-samsung-galaxy-zfold5-v1-2-9a91e635cacc@yahoo.com>

On 29.03.2024 12:08 AM, Alexandru Marc Serdeliuc via B4 Relay wrote:
> From: Alexandru Marc Serdeliuc <serdeliuk@yahoo.com>
> 
> Add support for Samsung Galaxy Z Fold5 (q5q) foldable phone
> 
> Currently working features:
> - Framebuffer
> - UFS
> - i2c
> - Buttons
> 
> Signed-off-by: Alexandru Marc Serdeliuc <serdeliuk@yahoo.com>
> ---

Looks like the commit message and contents got mixed up!

Konrad

^ permalink raw reply

* Re: [net-next,v4 0/2] ravb: Support describing the MDIO bus
From: patchwork-bot+netdevbpf @ 2024-03-29  1:30 UTC (permalink / raw)
  To: =?utf-8?q?Niklas_S=C3=B6derlund_=3Cniklas=2Esoderlund+renesas=40ragnatech=2E?=,
	=?utf-8?q?se=3E?=
  Cc: s.shtylyov, davem, edumazet, kuba, pabeni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, geert+renesas,
	claudiu.beznea.uj, yoshihiro.shimoda.uh, biju.das.jz, netdev,
	devicetree, linux-renesas-soc
In-Reply-To: <20240325153451.2366083-1-niklas.soderlund+renesas@ragnatech.se>

Hello:

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

On Mon, 25 Mar 2024 16:34:49 +0100 you wrote:
> Hello,
> 
> This series adds support to the binding and driver of the Renesas
> Ethernet AVB to described the MDIO bus. Currently the driver uses the OF
> node of the device itself when registering the MDIO bus. This forces any
> MDIO bus properties the MDIO core should react on to be set on the
> device OF node. This is confusing and non of the MDIO bus properties are
> described in the Ethernet AVB bindings.
> 
> [...]

Here is the summary with links:
  - [net-next,v4,1/2] dt-bindings: net: renesas,etheravb: Add optional MDIO bus node
    https://git.kernel.org/netdev/net-next/c/a87590c45c87
  - [net-next,v4,2/2] ravb: Add support for an optional MDIO mode
    https://git.kernel.org/netdev/net-next/c/2c60c4c008d4

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



^ permalink raw reply

* Re: [PATCH v2 2/6] arm64: dts: qcom: qcs6490-rb3gen2: Add DP output
From: Bjorn Andersson @ 2024-03-29  1:37 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, cros-qcom-dts-watchers, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
	Will Deacon, linux-arm-msm, devicetree, linux-kernel,
	linux-arm-kernel
In-Reply-To: <CAA8EJppCuoOnaB03GsjXGYSs5Q9iQ2uXHWQqfkPA5jKzdHc8NQ@mail.gmail.com>

On Thu, Mar 28, 2024 at 09:17:45AM +0200, Dmitry Baryshkov wrote:
> On Thu, 28 Mar 2024 at 05:07, Bjorn Andersson <andersson@kernel.org> wrote:
> >
> > On Thu, Mar 28, 2024 at 03:51:54AM +0200, Dmitry Baryshkov wrote:
> > > On Wed, 27 Mar 2024 at 04:04, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
> > > >
> > > > The RB3Gen2 board comes with a mini DP connector, describe this, enable
> > > > MDSS, DP controller and the PHY that drives this.
> > > >
> > > > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 40 ++++++++++++++++++++++++++++
> > > >  1 file changed, 40 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > > index 63ebe0774f1d..f90bf3518e98 100644
> > > > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > > @@ -39,6 +39,20 @@ chosen {
> > > >                 stdout-path = "serial0:115200n8";
> > > >         };
> > > >
> > > > +       dp-connector {
> > > > +               compatible = "dp-connector";
> > > > +               label = "DP";
> > > > +               type = "mini";
> > > > +
> > > > +               hpd-gpios = <&tlmm 60 GPIO_ACTIVE_HIGH>;
> > >
> > > Is it the standard hpd gpio? If so, is there any reason for using it
> > > through dp-connector rather than as a native HPD signal?
> > >
> >
> > I added it because you asked for it. That said, I do like having it
> > clearly defined in the devicetree.
> 
> I asked for the dp-connector device, not for the HPD function change.
> 

I didn't realize that you could have a dp-connector device without
defining the hpd-gpios, but it looks like you're right.

Do we have any reason for using the internal HPD, when we're already
spending the memory to allocate the dp-connector device?


PS. It's recommended that you dynamically switch to GPIO-based HPD in
lower-power scenarios, as this allow you to turn off the DP controller
and still detect plug events...

Regards,
Bjorn

^ 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