From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) (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 83E5038BF9C for ; Thu, 6 Aug 2026 07:19:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.45 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786000746; cv=none; b=OGT34AInZyW0FXc7kPj8MM2Lq13mJT16mf7eaWuLqN6zKTvxGDmSHG3+mbI56GcvqtLwvpUGbNaQjicUSLLtxpuMjf0T+vgPGyn+FkxNPSf+u8IQJq8sWf7P3UXer+hWKTDXlSNLh6gb3BsNGege3WYZ2Yhr4iZAONtRZcsJcrc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786000746; c=relaxed/simple; bh=pAnZTpbJUx3vZ+GsiQOgJ/n1CQp1O4Pq+UpZM1UMbH0=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=tT4f1BGtDRI4Oyd3xjlgmqBp4ozlrVnlf1WPcgAQlY78yiEn9rcQCvr+J+MPigV7NmXGNKArUgeX7OiFJwG2hfIpM2he9hQeF2d9oASzhCwM8mnurr6ipBhA3pzca+lLvVd0TXIPZFY+1dZWg5Io44Rzz2Ad2RPPOTeo0ZYc7dk= 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=L8PnNgG2; arc=none smtp.client-ip=209.85.128.45 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="L8PnNgG2" Received: by mail-wm1-f45.google.com with SMTP id 5b1f17b1804b1-496bb7cdf51so20676945e9.2 for ; Thu, 06 Aug 2026 00:19:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1786000743; x=1786605543; 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=J58Qoy4aHHn9awwFtSrKAEtr0Dz8WhHaRR0KQcTY5As=; b=L8PnNgG2b2VyMmFrYjdjNdDC3wEWoQY2eZDrcP7oFIj5QpQX6PivN0i2mmWOV1auGw hvwpezOBj5FIAWS6MqmczQZ40CFZOwV+pWPzV5fJn3AKnxlhy68S3RAJojtKbcrLiVaU uxxitkah+h2z1wBQE5/djgWjkpTTKP8rrHhyPjXNr64/lIL5OvD9SlahvDYxEAyqrIop kA5/Kq1BvDmN3B4mNv3UN85Aw4Ih3PENMlUUtHw2L8wguGfW30LuxHWTJKhGlfQW3212 HspQHzLrxbg/ghotlVvRX5+OMTDstojE2Gv5s3xNIfF9M9krb9G8rUS/Df3aQaax7Bfk j63w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786000743; x=1786605543; 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=J58Qoy4aHHn9awwFtSrKAEtr0Dz8WhHaRR0KQcTY5As=; b=bhhprU2yJCQ+fOZrjKAE/BOpVq+oTWMcuOyEEkRt3nxPx/u2YuE7seq0ijlRKHhK0a Sjz2zCx0piPEsiZ4alrZvTIM56iUdZxqutpRI4SpKiA+dn2mkUvLwHdzWfaYLLFMNfpN prk1e6Tskd8HKSrgi0KMqUmdjX6edlFfostmfNVpRTuKA2W+Ws4eQSzTl0sT119DGzWI Dci1pN9HWSH5eHZtkr3rzCdubfVx7pMe3WCP7bXNwjdBTJRGUM14pFdvcXqPB0gtxtfI MWaP6ADm6E5d9PJnkPhEQWzVaLuZfbBBC9pU5jcj9mxwmyC1atwySbKbbS/uZfPGvy06 6ujw== X-Forwarded-Encrypted: i=1; AHgh+RqBOLYlHQpWmJwfzLWehBRp6rcVapK0Vw35AaGzqxAv8P6iaQRYrDdNR/oJqeQTh6cD9LS+GuyZRT3wuH4=@vger.kernel.org X-Gm-Message-State: AOJu0YyKnr6EhNkyCOTm9uWEt38yn5sJja44WlkE2cn87iBbiZ8YBZ1T muDa6z5X2RfJtZLrvZOtcCQvCGGTjHwDzddlHZeaqPW0uLGREAXoEoKy X-Gm-Gg: AR+sD108CGkRqvPE4aQJmJBTfg9B4cAH1ktWkrB/jXeoUX1otCiF+bPOKs1bKZpt40J 3BCrN3G8fqmCGjnnkQhqWTzyh3swLpkhOOj08ICg2lyHR1DqPBpjDw/Cwk5DyIPI7IHUfOPIwkW 23LlkeV+ZGlPo4vt+emoiFEeXxBYistR1iBc4cJwChtACdBRerI3MjpyujWMOHAsdXPjnE8zWdf IivaaudKOsnltKSmNeJ7RMIfQr/sUZCRu1bJS4HawQxyicvwZBUXy4n43FKttzno6rGUIckeBci nTty1CBFIMcS5jPLvQbhPonspGw2nmPVpwc7zGJ2dARhSaw/idvhBf2XpisPbc9SGI2D1jV11pv aU7BRXNMTN7nt4r3hCfIiSQ4sthObqy15ACWwDHRc+Ha4+xUwOMtjO47UmDGZN+9vwABa3AuIvd 72ILM9IJn5wc8iZfI5PVtvkKYkUOSL0YjL6r2IXhKyaGxvczb3F9dZhsmgKQ2fbnnHfBONEkow/ rZy6qaf/c8OGoQhFrIgB1aKbn1BUu0gYsb8gHDL+/sZtkdG+MdGrewbGGuJjls= X-Received: by 2002:a05:600c:1d23:b0:499:51b8:d649 with SMTP id 5b1f17b1804b1-49951b8d652mr95522045e9.3.1786000742355; Thu, 06 Aug 2026 00:19:02 -0700 (PDT) Received: from ?IPV6:2001:9e8:f131:1d01:8da1:260c:d732:9394? ([2001:9e8:f131:1d01:8da1:260c:d732:9394]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4994e98fdedsm109323795e9.4.2026.08.06.00.19.00 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 06 Aug 2026 00:19:01 -0700 (PDT) Message-ID: Date: Thu, 6 Aug 2026 09:19:00 +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-next v11 1/4] dt-bindings: net: pse-pd: add bindings for Realtek PSE MCU Content-Language: en-US To: Jakub Kicinski Cc: o.rempel@pengutronix.de, kory.maincent@bootlin.com, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, pabeni@redhat.com, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, daniel@makrotopia.org, bjorn@mork.no, sander@svanheule.net References: <20260802100114.720594-2-jelonek.jonas@gmail.com> <20260806011107.1855326-1-kuba@kernel.org> From: Jonas Jelonek In-Reply-To: <20260806011107.1855326-1-kuba@kernel.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 06.08.26 03:11, Jakub Kicinski wrote: > This is an AI-generated review of your patch. The human sending this > email has considered the AI review valid, or at least plausible. > --- > dt-bindings: net: pse-pd: add bindings for Realtek PSE MCU > > This adds a new YAML binding for the MCU that fronts the PSE silicon on a > range of managed Realtek-based switches, covering both protocol generations > and the I2C/SMBus and UART attachments. Two questions below about the file > naming and about the serial slave node name. > >> diff --git a/Documentation/devicetree/bindings/net/pse-pd/realtek,pse-mcu-gen1.yaml b/Documentation/devicetree/bindings/net/pse-pd/realtek,pse-mcu-gen1.yaml >> new file mode 100644 >> index 0000000000000..3bb32349c28c4 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/pse-pd/realtek,pse-mcu-gen1.yaml >> @@ -0,0 +1,182 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/net/pse-pd/realtek,pse-mcu-gen1.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# > [ ... ] > >> +properties: >> + compatible: >> + oneOf: >> + # UART >> + - items: >> + - enum: >> + - zyxel,gs1900-10hp-a1-pse >> + - const: realtek,pse-mcu-gen1 >> + >> + # I2C, SMBus framing >> + - items: >> + - enum: >> + - zyxel,gs1920-24hp-v2-pse >> + - const: realtek,pse-mcu-gen1-smbus >> + >> + # UART >> + - items: >> + - enum: >> + - zyxel,gs1900-10hp-b1-pse >> + - zyxel,xmg1915-10ep-pse >> + - const: realtek,pse-mcu-gen2 >> + >> + # I2C, SMBus framing >> + - items: >> + - enum: >> + - zyxel,xs1930-12hp-pse >> + - const: realtek,pse-mcu-gen2-smbus >> + >> + # I2C, raw framing >> + - items: >> + - enum: >> + - linksys,lgs328mpc-v2-pse >> + - const: realtek,pse-mcu-gen2-i2c > [Severity: Low] > This isn't a bug, but should the file be named after the family rather than > after one compatible? > > The file (and $id) is realtek,pse-mcu-gen1.yaml, while the compatible list > here documents five strings across two protocol generations: > > realtek,pse-mcu-gen1 > realtek,pse-mcu-gen1-smbus > realtek,pse-mcu-gen2 > realtek,pse-mcu-gen2-smbus > realtek,pse-mcu-gen2-i2c > > There is no file bearing the name of any realtek,pse-mcu-gen2* compatible, > so searching by that compatible name finds nothing, and the commit message > does not mention that gen2 is described in a ...-gen1.yaml file. > > Would realtek,pse-mcu.yaml (with the $id updated to match) describe the > actual scope better? > > To be clear, the $id path itself is consistent with the current file path > (net/pse-pd/realtek,pse-mcu-gen1.yaml#), so $ref resolution and > dt_binding_check are not affected here - this is only about the declared > scope of the name. Could someone please take a decision on this and tell the bot how it should be? This is being raised again and again... Conor explicitly said the filename should carry one of the compatibles. >> +required: >> + - compatible >> + >> +allOf: >> + - $ref: pse-controller.yaml# > [ ... ] > >> + else: >> + allOf: >> + - $ref: /schemas/serial/serial-peripheral-props.yaml# >> + >> + properties: >> + reg: false > [Severity: Low] > This else branch declares the bare gen compatibles to be UART (serial > slave) attachments, and the node name is forced to ethernet-pse by > pse-controller.yaml: > > Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml > $nodename: > pattern: "^ethernet-pse(@.*|-([0-9]|[1-9][0-9]+))?$" > > But serial.yaml only enumerates these serial slave child node names: > > Documentation/devicetree/bindings/serial/serial.yaml > patternProperties: > "^(bluetooth|bluetooth-gnss|embedded-controller|gnss|gps|mcu|onewire)$": > if: > type: object > then: > additionalProperties: true > $ref: serial-peripheral-props.yaml# > ... > required: > - compatible > > Does a board device tree with an ethernet-pse child of serial then get the > shared slave-node subschema applied at all? Since serial.yaml ends with > additionalProperties: true, the unmatched child looks like it just falls > through silently rather than being flagged. It gets applied since it's referenced in my bindings. It just doesn't use the "routing" present in serial.yaml. > Should serial.yaml's pattern list gain ethernet-pse as part of this series, I would avoid this since it adds no value in what is validated. From what I've seen the pattern in serial.yaml isn't exhaustive so other bindings like "qca,qca7000" also use a node name not being in that list. > or is the intent to reuse the existing generic mcu name (which would > conflict with pse-controller.yaml's $nodename pattern)? No, reason already given in ( ). > [ ... ] > >> + # UART attachment >> + - | >> + serial { >> + ethernet-pse { >> + compatible = "zyxel,gs1900-10hp-a1-pse", "realtek,pse-mcu-gen1"; >> + current-speed = <19200>; > [ ... ] Regards, Jonas