From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2706737F01B for ; Wed, 7 Oct 2026 18:54:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1791399286; cv=none; b=l3yYCFKXf7vtSqtOFtWjakAGOaW2HFs8aWu7uxNSVIOWEH0bhmOjcrzCskms5M6xORYV5kg5l8z9Kkq8p/cirN+On9icr/b9KPrnDQVmMPVtrLWi8tvIIAdIPYG4DyZIkG39ikQgb/ZVvdYqMSxZCgjCjbfekP/kc3lIaxB0cCo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1791399286; c=relaxed/simple; bh=AK42eznXKQ8vp4qoeFBNkSEXkAz6wgJrds1tKvWlwmE=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=cQr8g+H5gJHqxhyDJPZMCQrkYe9E6iK40UxwmoYluE37gVB2LZbu0Nap0C+6T2IIesTA8dk1akrLWXTAai9j9UFWbMRJpGzr20KZbocH17NzvP0U/Ytwv+Q+cQqUzUbiWyUXFJArz47KO3Ltfdn3E/6oWdhIzVzBkjhzUqASPXw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=XQkuip6X; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=QugDWpLe; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="XQkuip6X"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="QugDWpLe" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1791399284; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=i/1Vqte/TZjvdrxy/PBXWIQttCIUhwAGvoe9ObRme4U=; b=XQkuip6XTvmknplbQJJmc3c8Xpn7pTBonSE+w4cY7aYFNi5HgfSY4LRoQ/M4yv5tk8XPsO vWG6wXuCk/cI0OTV2fj8Omsa/I5cGZ2Axlnegz77EF3QItQKZs2Zrqj7Pqcn0QzQLfaZK5 J5n8Hh8hX9hK6KcqJeVipVPbqf1Z4+Q= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-505-ceqc3i9HM2id3a2w4bW4kw-1; Wed, 07 Oct 2026 14:54:42 -0400 X-MC-Unique: ceqc3i9HM2id3a2w4bW4kw-1 X-Mimecast-MFC-AGG-ID: ceqc3i9HM2id3a2w4bW4kw_1791399281 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-4a01d1fb07bso45103725e9.2 for ; Wed, 07 Oct 2026 11:54:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1791399281; x=1792004081; darn=vger.kernel.org; h=content-transfer-encoding:content-type:in-reply-to:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to:content-type; bh=i/1Vqte/TZjvdrxy/PBXWIQttCIUhwAGvoe9ObRme4U=; b=QugDWpLepm+/C8jlpSnmtnnM47uPqpcMMbIUWd8pHtC4IeAFhruq4a25zyJBuwYv/X Lnn2nd0TA9j+EFELBq2VgA2KIXeSvfazsA08Q1xcj8EtqHFQvOTuvvEi5CFv1aTxqohD LM52mcj3mMkElD/3YPBg9dT32GpTxQFiTdxPeHYNTjgNAn/XQM32IKOLlpxKEGBTEMmq ++lLCDMfHxLivLZXGhj6X8tztDeTgHpEWGz1WJrjWoSo9QFB74VyD01G3rUhuh92qPN+ 4XxHCN3RibPRskNKL6PKwEvUBWPxHAeofhzWtnGNAivOej/Gslz4zdp37GH7MUnGccTG 2yCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1791399281; x=1792004081; h=content-transfer-encoding:content-type:in-reply-to:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=i/1Vqte/TZjvdrxy/PBXWIQttCIUhwAGvoe9ObRme4U=; b=f5hrrGADRAb9Lb5cuIqjQm0PdypldIAtn+Z2iD22LHQ7LbW0uRHR6cvxUWWh9NUKu0 2PCNmBfDL9D3ay9F9Kk9rnqwgA3+85ld82qnhL1Eum1Klj882OzG0osgK1TcmMHaGsHh N5s7EyZKRdrlKCbvJ1CJ7LfbNzAAZ2VNN0/c9wbpMltrhOko1pEqMbtGJn2K/IvbKdBq S6zjPaJdLDRv/mUT/+rgEPZMCkWM705X5Ll+MfMf85yR98pA4CBAbNCLLPMCLYhAWeES jKcRbzIYylZQrB9dWL+Pj9+GsYN1b9kRD6Y9PAYNaRkNCdcS46Ip4c3k1o+grvPXNiqo Qcjw== X-Forwarded-Encrypted: i=1; AKwUvBxkdGa0+SO1bIwcNO+OcXSex80upXBATntsPSEpJpd7BdQJN9NyLvMVMk3+qG6XfMJXx5k8ldF9tcnltlw=@vger.kernel.org X-Gm-Message-State: AFuF++mY371ve0R2PJHk2ovb3cUF93bZc8Q46c0yvB0IEjtIHIoA9V4O HAeMJwacAqo+c4YMFPRQWjAa3XoLI4zsFPaAx9kk89CKQlzBJ5tjPyESZIJ/y3qqWO2xaon1Cme khqzyIMUz/wZNwr/eC1PE32If+nt9G8L34jyZNCaj1OBUszpN9rc3bwvIoi0kssUqog== X-Gm-Gg: AYBFou18LhDb2CD/HIm9hLsOuW//VAir6GoTJA/JeXz658jiJDXxEpOYHBB9UvD/dup iG+7ANhMSsZOQ6QWL+Wn385pFsnI7S4c9vF9Z60yU/f0WypBrC8+CSqMICyeo7tiVZHqAbX6B+6 P34bE6Ma7hlDZy26PuPen1/zi9jDHA5NHou4eGBvNIAIPFQ6WXR9O2BdLvkI/owIOLWOkkjGi3U E/4mPc7cnbdV8OAga6At1Zb29PZ2/OA3rwU5ezgQDmSAgNyrcL//CnjF5LsWaaY2kmYuTsZDRX5 DI3u73+D3w7Y7J+t7m2OHsVNLcN+zhCabHtqgDL8oFhQxuYAjNN679J91A5EH9c4/iygi+MLhA= = X-Received: by 2002:a05:600c:64c6:b0:49f:fe90:de63 with SMTP id 5b1f17b1804b1-4a180455e85mr59169935e9.28.1791399281257; Wed, 07 Oct 2026 11:54:41 -0700 (PDT) X-Received: by 2002:a05:600c:64c6:b0:49f:fe90:de63 with SMTP id 5b1f17b1804b1-4a180455e85mr59169625e9.28.1791399280814; Wed, 07 Oct 2026 11:54:40 -0700 (PDT) Received: from [192.168.2.75] ([46.175.183.46]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4a18445b6b9sm9971375e9.8.2026.10.07.11.54.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 07 Oct 2026 11:54:39 -0700 (PDT) Message-ID: Date: Wed, 7 Oct 2026 20:54:38 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net v3 2/4] dpll: zl3073x: make embedded sync follow the output carrier frequency Content-Language: en-US To: Ivan Vecera , netdev@vger.kernel.org Cc: Min Li , Vadim Fedorenko , Arkadiusz Kubalewski , Jiri Pirko , Jakub Kicinski , Prathosh Satish , Paolo Abeni , linux-kernel@vger.kernel.org References: <20261006153116.347497-1-ivecera@redhat.com> <20261006153116.347497-3-ivecera@redhat.com> From: Petr Oros In-Reply-To: <20261006153116.347497-3-ivecera@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 10/6/26 5:31 PM, Ivan Vecera wrote: > Changing an output pin's frequency did not update its embedded sync > (eSync) configuration. The eSync period and width are computed relative > to the carrier: > > esync_n_period = out_freq / esync_freq > esync_n_width = div / 2 > > Once zl3073x_dpll_output_pin_frequency_set() changes div, both values no > longer match the new carrier, so the embedded sync runs at the wrong > frequency and duty cycle. Worse, if the new carrier is <= 1 Hz then > esync_get() returns -EOPNOTSUPP (out_freq <= 1), hiding the stale ESYNC > mode so the user can no longer turn it off - it is stuck on. > > This was hit during development: after changing an output pin's > frequency the embedded sync output stopped working correctly, as > confirmed on an oscilloscope - it ran at the wrong frequency and duty > cycle because the eSync period and width still matched the old carrier. > For a new 1 Hz carrier esync_get() returned -EOPNOTSUPP, leaving eSync > enabled with no way to disable it. > > Factor the eSync handling into zl3073x_out_esync_{is_enabled,enable, > disable}() helpers and reuse them from esync_get()/esync_set(), then > adjust the eSync parameters in the output frequency change path for > non-N-divided outputs after updating div/width: > > - If the new frequency is 1 Hz, disable eSync. It cannot work at a 1 Hz > carrier and esync_get() hides it, so it must be cleared rather than > left stuck on. > - Otherwise, if eSync is active, recompute its period and width for the > new carrier keeping the 1 Hz eSync frequency. > > As a side effect of reusing the helpers, esync_get() now reports the > driver's eSync model - a 1 Hz frequency and a 25% pulse - from the clock > type when eSync is enabled, instead of deriving the values from the > cached esync_n_period/esync_n_width registers. The driver only ever > programs a 1 Hz / 25% eSync, so this matches what esync_set() configures > and keeps get and set symmetric. A configuration the driver did not > establish - a non-1 Hz eSync left in flash or by an older kernel, or the > truncated duty cycle of an odd divider - is therefore reported as the > nominal 1 Hz / 25% rather than its exact register values. > > N-divided outputs are left untouched: there esync_n_period is the N > divider and eSync is not supported anyway. > > Fixes: ce26d7ca50a5 ("dpll: zl3073x: Add support to get/set frequency on pins") > Fixes: 634ca2cb06d2 ("dpll: zl3073x: Add support to get/set esync on pins") > Signed-off-by: Ivan Vecera > --- > drivers/dpll/zl3073x/dpll.c | 68 ++++++++++++++++--------------------- > drivers/dpll/zl3073x/out.h | 36 ++++++++++++++++++++ > 2 files changed, 66 insertions(+), 38 deletions(-) > > diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c > index f2e137475b40..7c997966c3c3 100644 > --- a/drivers/dpll/zl3073x/dpll.c > +++ b/drivers/dpll/zl3073x/dpll.c > @@ -852,7 +852,7 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin, > struct zl3073x_dpll_pin *pin = pin_priv; > const struct zl3073x_synth *synth; > const struct zl3073x_out *out; > - u32 synth_freq, out_freq; > + u32 synth_freq; > u8 out_id; > > guard(mutex)(&zldpll->lock); > @@ -864,38 +864,28 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin, > * for N-division is also used for the esync divider so both cannot > * be used. > */ > - if (zl3073x_out_is_ndiv(out)) > + if (zl3073x_out_is_ndiv(out) || !pin->esync_control) > return -EOPNOTSUPP; > > /* Get attached synth frequency */ > synth = zl3073x_synth_state_get(zldev, zl3073x_out_synth_get(out)); > synth_freq = zl3073x_synth_freq_get(synth); > - out_freq = synth_freq / out->div; > > - if (!pin->esync_control || out_freq <= 1) > + /* The esync is not supported for 1 Hz base frequency */ > + if (synth_freq / out->div <= 1) > return -EOPNOTSUPP; > > esync->range = esync_freq_ranges; > esync->range_num = ARRAY_SIZE(esync_freq_ranges); > > - if (zl3073x_out_clock_type_get(out) != ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC) { > - /* No need to read esync data if it is not enabled */ > + if (zl3073x_out_esync_is_enabled(out)) { > + esync->freq = 1; > + esync->pulse = 25; > + } else { > esync->freq = 0; > esync->pulse = 0; > - > - return 0; > } > > - /* Compute esync frequency */ > - esync->freq = out_freq / out->esync_n_period; > - > - /* By comparing the esync_pulse_width to the half of the pulse width > - * the esync pulse percentage can be determined. > - * Note that half pulse width is in units of half synth cycles, which > - * is why it reduces down to be output_div. > - */ > - esync->pulse = (50 * out->esync_n_width) / out->div; > - > return 0; > } > > @@ -926,32 +916,17 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin, > if (zl3073x_out_is_ndiv(&out)) > return -EOPNOTSUPP; > > - /* Update clock type in output mode */ > - if (freq) > - zl3073x_out_clock_type_set(&out, > - ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC); > - else > - zl3073x_out_clock_type_set(&out, > - ZL_OUTPUT_MODE_CLOCK_TYPE_NORMAL); > - > - /* If esync is being disabled just write mailbox and finish */ > - if (!freq) > + if (!freq) { > + zl3073x_out_esync_disable(&out); > return zl3073x_out_state_set(zldev, out_id, &out); > + } > > /* Get attached synth frequency */ > synth = zl3073x_synth_state_get(zldev, zl3073x_out_synth_get(&out)); > synth_freq = zl3073x_synth_freq_get(synth); > > - /* Compute and update esync period */ > - out.esync_n_period = synth_freq / (u32)freq / out.div; > - > - /* Half of the period in units of 1/2 synth cycle can be represented by > - * the output_div. To get the supported esync pulse width of 25% of the > - * period the output_div can just be divided by 2. Note that this > - * assumes that output_div is even, otherwise some resolution will be > - * lost. > - */ > - out.esync_n_width = out.div / 2; > + /* Enable 1PPS eSync for this pin frequency */ > + zl3073x_out_esync_enable(&out, synth_freq / out.div); > > /* Commit output configuration */ > return zl3073x_out_state_set(zldev, out_id, &out); > @@ -1009,6 +984,23 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin, > /* For 50/50 duty cycle the divisor is equal to width */ > out.width = new_div; > > + /* The embedded sync period and width are computed relative to > + * the output carrier so they have to be adjusted to follow the > + * new frequency. > + */ > + if (zl3073x_out_esync_is_enabled(&out)) { > + if (frequency == 1) { > + /* Disable eSync if the new frequency is 1 Hz */ > + zl3073x_out_esync_disable(&out); > + } else { > + /* Update esync period and width according to > + * the new frequency. > + */ > + zl3073x_out_esync_enable(&out, > + synth_freq / out.div); > + } > + } > + > /* Commit output configuration */ > return zl3073x_out_state_set(zldev, out_id, &out); > } > diff --git a/drivers/dpll/zl3073x/out.h b/drivers/dpll/zl3073x/out.h > index 660889c57bff..d2b8b4eb5a76 100644 > --- a/drivers/dpll/zl3073x/out.h > +++ b/drivers/dpll/zl3073x/out.h > @@ -134,4 +134,40 @@ static inline u8 zl3073x_out_synth_get(const struct zl3073x_out *out) > return FIELD_GET(ZL_OUTPUT_CTRL_SYNTH_SEL, out->ctrl); > } > > +static inline bool zl3073x_out_esync_is_enabled(const struct zl3073x_out *out) > +{ > + u8 clock_type = zl3073x_out_clock_type_get(out); > + > + return clock_type == ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC; > +} > + > +static inline > +void zl3073x_out_esync_enable(struct zl3073x_out *out, u32 base_freq) > +{ > + /* Period is expressed as the number of output divider clock cycles. > + * So for 1 Hz the period is equal to base frequency. > + */ > + out->esync_n_period = base_freq; > + > + /* Half of the period in units of 1/2 synth cycle can be represented by > + * the output_div. To get the supported esync pulse width of 25% of the > + * period the output_div can just be divided by 2. Note that this > + * assumes that output_div is even, otherwise some resolution will be > + * lost. > + */ > + out->esync_n_width = out->div / 2; > + > + /* Update the clock type */ > + zl3073x_out_clock_type_set(out, ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC); > +} > + > +static inline > +void zl3073x_out_esync_disable(struct zl3073x_out *out) > +{ > + /* Just update the clock type. The eSync period and width are > + * ignored for this clock type. > + */ > + zl3073x_out_clock_type_set(out, ZL_OUTPUT_MODE_CLOCK_TYPE_NORMAL); > +} > + > #endif /* _ZL3073X_OUT_H */ Reviewed-by: Petr Oros