| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c56d03cb-14a4-4a7f-82e7-80368c3ca4ec@gmail.com>
Date: Wed, 7 May 2025 11:49:06 +0300
From: Cosmin Tanislav <demonsingur@...il.com>
To: Jakub Kostiw <jakub.kostiw@...etronic.com>
Cc: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>,
Cosmin Tanislav <cosmin.tanislav@...log.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Niklas Söderlund
<niklas.soderlund@...natech.se>, Julien Massot
<julien.massot@...labora.com>, Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski
<brgl@...ev.pl>, Bjorn Andersson <quic_bjorande@...cinc.com>,
Geert Uytterhoeven <geert+renesas@...der.be>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, Arnd Bergmann
<arnd@...db.de>, Taniya Das <quic_tdas@...cinc.com>,
Biju Das <biju.das.jz@...renesas.com>,
Nícolas F . R . A . Prado <nfraprado@...labora.com>,
Eric Biggers <ebiggers@...gle.com>,
Javier Carrasco <javier.carrasco@...fvision.net>,
Ross Burton <ross.burton@....com>, Hans Verkuil <hverkuil@...all.nl>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Zhi Mao <zhi.mao@...iatek.com>,
Kieran Bingham <kieran.bingham@...asonboard.com>,
Dongcheng Yan <dongcheng.yan@...el.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Benjamin Mugnier <benjamin.mugnier@...s.st.com>,
Tommaso Merciai <tomm.merciai@...il.com>,
Dan Carpenter <dan.carpenter@...aro.org>,
Ihor Matushchak <ihor.matushchak@...box.net>,
Laurentiu Palcu <laurentiu.palcu@....nxp.com>, linux-media@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-staging@...ts.linux.dev,
linux-gpio@...r.kernel.org
Subject: Re: [RFC PATCH v2 12/16] media: i2c: add Maxim GMSL2/3 serializer and
deserializer drivers
On 5/7/25 10:28 AM, Jakub Kostiw wrote:
>> Can you revert the change you made to polarity_on_physical_lanes, and
>> try the following?
>>
>> diff --git a/drivers/media/i2c/maxim-serdes/max9296a.c b/drivers/
>> media/i2c/maxim-serdes/max9296a.c
>> index f48f5b68a750..dea0518fd790 100644
>> --- a/drivers/media/i2c/maxim-serdes/max9296a.c
>> +++ b/drivers/media/i2c/maxim-serdes/max9296a.c
>> @@ -474,7 +474,7 @@ static int max9296a_init_phy(struct max_des *des,
>> struct max_des_phy *phy)
>> */
>>
>> if (priv->info->polarity_on_physical_lanes)
>> - map = phy->mipi.data_lanes[i];
>> + map = phy->mipi.data_lanes[i] - 1;
>> else
>> map = i;
>>
>> data_lanes is 1-based (since 0 is the clock lane), but the bits
>> in register 0x335 start from 0. That means we should adjust the
>> values in data_lanes to be 0-based.
>
> I have applied your patch and polarity settings seems to be correct now
> (based on register contents).
> However, I have came across another issue.
> When I was debugging the driver for MAX96714, before I found out that
> the issue was with polarity settings, I have commented out calls to
> MAX9296A_DPLL_0. Probably because I thought it was there by mistake. I
> totally forgot about that change.
> Before applying your patch I reverted any changes to the driver, so
> MAX9296A_DPLL_0 writes were back again. Sadly, video stream did not
> work. So I began to wonder, and just for sake of testing, commented
> these calls again (added some logs for quick tracing purposes):
>
> diff --git a/drivers/media/i2c/maxim-serdes/max9296a.c b/drivers/media/
> i2c/maxim-serdes/max9296a.c
> index f48f5b68a..b24a8e2d6 100644
> --- a/drivers/media/i2c/maxim-serdes/max9296a.c
> +++ b/drivers/media/i2c/maxim-serdes/max9296a.c
> @@ -391,6 +391,8 @@ static int max9296a_init_phy(struct max_des *des,
> struct max_des_phy *phy)
> * PHY1 Lane 1 = D3
> */
>
> + dev_info(priv->dev, "Using %d lanes", num_data_lanes);
> +
> /* Configure a lane count. */
> ret = regmap_update_bits(priv->regmap,
> MAX9296A_MIPI_TX10(hw_index),
> MAX9296A_MIPI_TX10_CSI2_LANE_CNT,
> @@ -474,7 +476,7 @@ static int max9296a_init_phy(struct max_des *des,
> struct max_des_phy *phy)
> */
>
> if (priv->info->polarity_on_physical_lanes)
> - map = phy->mipi.data_lanes[i];
> + map = phy->mipi.data_lanes[i] - 1;
> else
> map = i;
>
> @@ -484,6 +486,9 @@ static int max9296a_init_phy(struct max_des *des,
> struct max_des_phy *phy)
> if (phy->index == 0 && priv->info->phy0_lanes_0_1_on_second_phy)
> val = ((val & 0x3) << 2) | ((val >> 2) & 0x3);
>
> + dev_info(priv->dev, "Val for MIPI_PHY5 (0_1): %lx",
> FIELD_PREP(MAX9296A_MIPI_PHY5_PHY_POL_MAP_0_1, val));
> + dev_info(priv->dev, "Val for MIPI_PHY5 (2_3): %lx",
> FIELD_PREP(MAX9296A_MIPI_PHY5_PHY_POL_MAP_2_3, val >> 2));
> +
> ret = regmap_update_bits(priv->regmap, MAX9296A_MIPI_PHY5(index),
> MAX9296A_MIPI_PHY5_PHY_POL_MAP_0_1 |
> MAX9296A_MIPI_PHY5_PHY_POL_MAP_2_3,
> @@ -499,10 +504,10 @@ static int max9296a_init_phy(struct max_des *des,
> struct max_des_phy *phy)
> return ret;
>
> /* Put DPLL block into reset. */
> - ret = regmap_clear_bits(priv->regmap, MAX9296A_DPLL_0(hw_index),
> - MAX9296A_DPLL_0_CONFIG_SOFT_RST_N);
> - if (ret)
> - return ret;
> + //ret = regmap_clear_bits(priv->regmap, MAX9296A_DPLL_0(hw_index),
> + // MAX9296A_DPLL_0_CONFIG_SOFT_RST_N);
> + //if (ret)
> + // return ret;
>
> /* Set DPLL frequency. */
> ret = regmap_update_bits(priv->regmap, MAX9296A_BACKTOP22(index),
> @@ -519,10 +524,10 @@ static int max9296a_init_phy(struct max_des *des,
> struct max_des_phy *phy)
> return ret;
>
> /* Pull DPLL block out of reset. */
> - ret = regmap_set_bits(priv->regmap, MAX9296A_DPLL_0(index),
> - MAX9296A_DPLL_0_CONFIG_SOFT_RST_N);
> - if (ret)
> - return ret;
> + //ret = regmap_set_bits(priv->regmap, MAX9296A_DPLL_0(index),
> + // MAX9296A_DPLL_0_CONFIG_SOFT_RST_N);
> + //if (ret)
> + // return ret;
>
> if (dpll_freq > 1500000000ull) {
> /* Enable initial deskew with 2 x 32k UI. */
>
> To my surprise it works this way. I tested this 2 times back and forth.
> Can these calls really cause some issues?
Can you revert your changes and try this?
diff --git a/drivers/media/i2c/maxim-serdes/max9296a.c
b/drivers/media/i2c/maxim-serdes/max9296a.c
index dea0518fd790..3bb80fe42a22 100644
--- a/drivers/media/i2c/maxim-serdes/max9296a.c
+++ b/drivers/media/i2c/maxim-serdes/max9296a.c
@@ -519,7 +519,7 @@ static int max9296a_init_phy(struct max_des *des,
struct max_des_phy *phy)
return ret;
/* Pull DPLL block out of reset. */
- ret = regmap_set_bits(priv->regmap, MAX9296A_DPLL_0(index),
+ ret = regmap_set_bits(priv->regmap, MAX9296A_DPLL_0(hw_index),
MAX9296A_DPLL_0_CONFIG_SOFT_RST_N);
if (ret)
return ret;
Powered by blists - more mailing lists