From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6667947884A for ; Wed, 19 Aug 2026 14:28:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787149700; cv=none; b=dcHC6vKdFGXP1b8q7DVIlkBNgIzu1GfAd2f7YB0lR9FUrZEl2+6z0n/BmMdkepPqjZYmssL4mfEhO+EKh9zO1V8DL4vmEM8W6OHpzDC35G7cbZPv1n0SmJuRjywde78tfaIzsVDB7vgHFv1tbIbGy+3YbwmI/GDDEkq4UoC4LsA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787149700; c=relaxed/simple; bh=NdR/r5cs4L7ZRKp84zLL5vTM42zkOs6f/D8Mq7IpFms=; h=Mime-Version:Content-Type:Date:Message-Id:Cc:Subject:From:To: References:In-Reply-To; b=dXeuqvaS40qY107XRUB67v9zsUwoEdUGit5DYqvhC1pOMXP+SV46xVcPKwlrFnTDsOxcbKMGrVagdKcnFLXGY5HTbL6LsmhWEKvlwf4zepLveVO8FL2PH+ev1RgDaFwTPDJdQtwejGa1es1jaxgfKdRXoaz6DBUYx/QBQN6OxNg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=QxLIa+Oi; arc=none smtp.client-ip=209.85.128.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="QxLIa+Oi" Received: by mail-wm1-f41.google.com with SMTP id 5b1f17b1804b1-4954a9e8490so13972485e9.1 for ; Wed, 19 Aug 2026 07:28:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1787149687; x=1787754487; darn=vger.kernel.org; h=in-reply-to:references:to:from:subject:cc:message-id:date :content-type:content-transfer-encoding:mime-version:from:to:cc :subject:date:message-id:reply-to:content-type; bh=7un4O3L9YLxIWNurYwHJNMBBiutfCLK9e9so1V1Jlvw=; b=QxLIa+OitKCiehiaW8mxC+S8O9Bk0pLa8p+pHr0hJLiHIUrdvCUDp88yASlfqcV1rl iDACjBiExOwmH3OOSyiszg8ajWnQz7F0aYhZJpe9PrGDFixXOPZUBRrhduN9Sphuhk26 BiETZ0eeD28hElYFPhnQ6yocFtdcOHSRbaH4L9iiqiz6gSRxmOo9tmWbKTNQHIyy4y/3 ka1FJPYOiOgeGckslYXMo+WNqzNnoBlSaN52ZJc3cImwmMBjobxZ7w3noRdc+awt7SFN f4KR5jVT2gG9M6FlE2HN8lgGRFGhv/qJgWc7/mxn1E2zZpdbqvdYhaoAxVQxCqTl+GoQ 46uA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1787149687; x=1787754487; h=in-reply-to:references:to:from:subject:cc:message-id:date :content-type:content-transfer-encoding:mime-version:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=7un4O3L9YLxIWNurYwHJNMBBiutfCLK9e9so1V1Jlvw=; b=DfjtVeBu6qUQS3bp64d2gnzW5u2moDB3f1nLc17DtWc0v8RcxYgIDBbm/FjjVqeTSy gHparGSRhOtuZ+7BaOX1Y+xmbW6TunX9ZTBAENvCXKEPlB1ZBbhwEosDy0JsGmKZaB+o 6ktknI65d5qMKGc8XW+sKsBfkByVNCHDATkeSOY1gRI1+4vVhLe6KxKF6O8OPRWbVqSd SJTGedf6sxhjkgHDZ7gHVdPjHG1pFUyqSlqfJNTJDJSQ40HM3Vh6EM5LFgNii0hs5cIP cFlGpwNcZgvIGxeM2FDq4qahAlAJc6CPcVhqigwIVP+jBZJKdOsV20MCoNz3peqSsDEz W6vw== X-Forwarded-Encrypted: i=1; AHgh+Rok2iXy+UOjkJyWSzvqYn/xlPOiF6r/2bbJInP2mv3NuoOnhJfgI9yQpWmghoXQJtLEr1S5/VDUWHjfBDo=@vger.kernel.org X-Gm-Message-State: AOJu0YxOcmkW62r4Dzw4CDO4prxKBNPETHvWZLnO8wi6fjJg9xtJfh1O a9/WSCD5CWGZIZhEHSdTmjT0HJPNnYKWcbCfEDG3ZTDo7+JhD2+c5C0nBjEdG+vc X-Gm-Gg: AR+sD11OiED0vKq7lSh684M6WpcXsA3pqM6g90zlXDTxWL7WWgsAOz7gQJNCaUM1p4d pujgQWIstKumW3qIZAOTZmezlCnCBq1a3SeKT3q6vQTpHeqTWkLeu837NQnLe7GrQmFMezRQFJo ptCDkQD+7jB9Drx08gpjhdZRLUXmMPBEcGpwu55QyxtiMJ+xw5fT5xKbOlEeLXG90RZA+Igyg+q 6QT2QfoXCXTsEp3mJF/+EvnTcu8QeJRdzIv4/PHhUk94luT6L8mWn0G7U1AcCR1ehwXGhkAQJix 5zMOl7j27aXmgUwMVqX0Tmpq3rx5a1v11kgUhRSZiQWd5lx6lfrgRIgavDIS61HKjSYUKlTbr1K WfucDHXxBUBfLsVtSVVEFSBJUYfPOY2dkPx3xKqwLuKs+uTsBNG+FRyTqR1wZ9UsZfUi9YnoXex QJU61jTlTrjJCk++D6maGaRx8ZQDfx6y44RNSFguE+fidE3buBVtEdJLJ2jVhcYUPxHUgE6epXV M+nhVfNYo2cd7ICwGD45CUx++vi2xemBfamhVZwoiofJABJ77dErkI= X-Received: by 2002:a05:600c:8b71:b0:499:5f80:83ac with SMTP id 5b1f17b1804b1-499aa100de8mr89119795e9.7.1787149686732; Wed, 19 Aug 2026 07:28:06 -0700 (PDT) Received: from localhost (178.43.142.119.ipv4.supernova.orange.pl. [178.43.142.119]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-499a9ddee1esm39709255e9.2.2026.08.19.07.28.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 19 Aug 2026 07:28:05 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 19 Aug 2026 16:21:14 +0200 Message-Id: Cc: , , , , , , =?utf-8?q?Niklas_S=C3=B6derlund?= , "Martin Hecht" , "Andrian Suciu" , "Cosmin Tanislav" Subject: Re: [PATCH v15 16/22] media: i2c: maxim-serdes: add MAX96717 driver From: "Artur Andrzejczak" To: "Dumitru Ceclan" , "Tomi Valkeinen" , "Mauro Carvalho Chehab" , "Sakari Ailus" , "Laurent Pinchart" , "Julien Massot" , "Rob Herring" , =?utf-8?q?Niklas_S=C3=B6derlund?= , "Greg Kroah-Hartman" X-Mailer: aerc 0.21.0 References: <20260807-gmsl2-3_serdes-v15-0-7212e9e5156a@analog.com> <20260807-gmsl2-3_serdes-v15-16-7212e9e5156a@analog.com> In-Reply-To: <20260807-gmsl2-3_serdes-v15-16-7212e9e5156a@analog.com> I only went through the clock, pinctrl and CSI-2 lane code, not the whole patch. On Fri Aug 7, 2026 at 3:02 PM CEST, Dumitru Ceclan wrote: > [...] > + { "maxim,jitter-compensation", MAX96717_PINCTRL_JITTER_COMPENSATION_EN,= 0 }, The automated review already raised this: maxim,jitter-compensation is a boolean in the binding, so the property is present but zero length. Reading a zero-length property as a u32 gives -EOVERFLOW. pinconf-generic treats only -EINVAL as absent and substitutes the default above on any other error, so 0 lands. max96717_conf_pin_config_set_one() then takes the arg ? en_val : ~en_val branch with arg =3D=3D 0 and clears the bit. That means the property for enabling jitter compensation never sets this bit, and nothing else in the driver sets it either. Giving it a value does not help, since maxim,jitter-compensation =3D <1> is rejected by the schema. pinctrl-k210.c uses 1 for its boolean params. Should the default value here be 1 as well? > [...] > + /* Configure a lane count. */ > + ret =3D regmap_update_bits(priv->regmap, MAX96717_MIPI_RX1, > + MAX96717_MIPI_RX1_CTRL_NUM_LANES, > + FIELD_PREP(MAX96717_MIPI_RX1_CTRL_NUM_LANES, > + num_data_lanes - 1)); max96717_init_phy() calculates num_data_lanes - 1 without checking for zero. The binding requires data-lanes, so this needs a non-conforming DT, but the driver removed in patch 21 still rejected it with "Invalid data lanes must be 1 to 4". Here num_data_lanes remains 0 and the write sets 0b11, so the count becomes four, silently. max_ser_find_phys_config() compares num_data_lanes against the single { 4 } entry in max96717_phys_configs, so zero passes there too. The automated review asked about the upper bound on this line, this is the lower one. Shall the range check come back? > [...] > + val =3D FIELD_PREP(MAX96717_REF_VTG0_REFGEN_PREDEF_FREQ, > + predef_freq->val); > + > + if (predef_freq->is_alt) > + val |=3D MAX96717_REF_VTG0_REFGEN_PREDEF_FREQ_ALT; > + if (!predef_freq->is_rclk) > + val |=3D MAX96717_REF_VTG0_REFGEN_EN; > + > + val |=3D MAX96717_REF_VTG0_REFGEN_RST; > + > + ret =3D regmap_write(priv->regmap, MAX96717_REF_VTG0, val); max96717_clk_set_rate() sets REFGEN_PREDEF_FREQ and its ALT bit, but not the predefined frequency enable bit. The REF_VTG0 defines stop at PREDEF_FREQ. Bit 6 is not part of the composed value, and this is a full regmap_write(), so the write clears it. The driver removed in patch 21 sets REFGEN_PREDEF_EN (BIT(6)) in its own write of that register, with the same encodings for the six rates the two tables share. Currently, that bit is left 0 on every rate that goes through REFGEN, including the 24 MHz default programmed at probe. The datasheet has REFGEN_PREDEF_EN reset to 1, and describes REF_VTG4/5 as the feedback divider fraction used when predefined mode is disabled, which this driver never writes. Was that on purpose, or should it be set here too? > [...] > +static int max96717_register_clkout(struct max96717_priv *priv) > +{ > + struct device *dev =3D &priv->client->dev; > + struct clk_init_data init =3D { .ops =3D &max96717_clk_ops }; > + int ret; > + > + ret =3D max96717_mux_set_rclkout(priv, MAX96717_RCLK_MFP); > + if (ret) > + return ret; max96717_register_clkout() routes RCLKOUT to mfp4 and sets that pin to the fastest slew rate. It runs after max96717_gpiochip_probe(), which calls pinctrl_enable(), so at that time the pin setup from the max96717 node itself has already been applied. The binding in patch 03 allows function =3D "rclkout" on mfp2, so a config that asks for the RCLK there gets it, but loses it a few lines later: max96717_mux_set_rclkout() clears RCLK_ALT for any group other than mfp2. A slew rate set on mfp4 is overwritten the same way. Should this leave alone a mux state that pinctrl has already selected? Moreover, the driver names seven slew fields for eleven pins. The binding allows slew-rate on every pin, but max96717_get_pin_config_reg() returns -EINVAL for it on mfp5, mfp6, mfp9 and mfp10, so a slew-rate that passes the schema on one of those four makes pinctrl_select_state() fail. Another device pointing its pinctrl-0 at such a node fails to probe, since pinctrl_bind_pins() returns -EINVAL. On the max96717 node itself it does not: pinctrl_claim_hogs() logs the error and returns 0. Either way pinctrl_commit_state() stops at the failing setting, so the pin configs after it are never applied. Table 13 in the datasheet lists no pin slew for mfp5, mfp6, mfp9 and mfp10, so the driver looks right here. Should the binding restrict slew-rate to the seven pins that have the field? Kind Regards, Artur Andrzejczak