From mboxrd@z Thu Jan 1 00:00:00 1970 List-Id: Return-Path: Received: from localhost (localhost [127.0.0.1]) by mail.ignore.pl (Postfix) with ESMTP id C17EA401AD; Thu, 20 Nov 2025 11:23:44 +0000 (UTC) X-Virus-Scanned: Debian amavis at ignore.pl Received: from mail.ignore.pl ([127.0.0.1]) by localhost (geidontei.ignore.pl [127.0.0.1]) (amavis, port 10024) with ESMTP id 36UEoG3TjGho; Thu, 20 Nov 2025 11:23:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ignore.pl; s=gdt; t=1763637822; bh=VhC0c9c6GBHlqtal9u3WQkEYtpjlBk9esbZGmhVvWc0=; h=Date:Subject:To:References:Cc:From:In-Reply-To:From; b=e3byCNX+tOJmpih1tLhIDLMVr1wdtCIwltCvjeI9LW8nTNBPV4TfFrVVzxb3hXlJ5 PWnca8iLcy2H/1hsnZZAvJEatUhJJjv2WwC4FulOEMsAzHxqYJDAvhL1WkfIGh5F3F Ut5e8cjJDw4FBTwGZMUsmfTvQkNNcWan71AmqDU09SMWoVsxVBGV3U8CgEVGfVmwzH X+5L+ekxToPRluhCZHPMZmobTvM5eS6290VwCv8IdaEO0CjTCu4sCQ8oLwZd8rN1Lw NI+ibgDEjo8kpr+eiDKFv0e+8ipA+3CWaidhyqguI1xhPQLqfQwlWqx98RXEavx8Mb dk+qO//iuq39w== Received: from [192.168.1.9] (daw118.neoplus.adsl.tpnet.pl [83.23.22.118]) by mail.ignore.pl (Postfix) with ESMTPSA id A7C25401BC; Thu, 20 Nov 2025 11:23:42 +0000 (UTC) Message-ID: Date: Thu, 20 Nov 2025 12:23:41 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH what RFC] ice: implement support for 0x0601 opcode To: Dawid Osuchowski References: <20251120105349.5854-1-dawid.osuchowski@linux.intel.com> Content-Language: pl-PL, en-GB Cc: patches@ignore.pl From: Aki In-Reply-To: <20251120105349.5854-1-dawid.osuchowski@linux.intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 20/11/2025 11:53, Dawid Osuchowski wrote: > Signed-off-by: Dawid Osuchowski > --- > PHY Type support is a little wonky and based on 0x0607, but I wanted to get this patch out for comments. Of course. Thank you for sending a patch. > --- > samples/linux_0601.log | 12 ++ > what/ice/opcodes/0x0601.lua | 238 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 250 insertions(+) > create mode 100644 samples/linux_0601.log > create mode 100644 what/ice/opcodes/0x0601.lua > > diff --git a/samples/linux_0601.log b/samples/linux_0601.log > new file mode 100644 > index 000000000000..23dee128ff59 > --- /dev/null > +++ b/samples/linux_0601.log > @@ -0,0 +1,12 @@ > +[ 5.000000] ice 0000:01:00.0: AQ Command: opcode 0x0601, flags 0x3400, datalen 0x0018, retval 0x0000 > + cookie (h,l) 0x00000000 0x00000000 > + param (0,1) 0x00000000 0x00000000 > + addr (h,l) 0x00000004 0x3CB75000 > +[ 5.000000] ice 0x00000004 0x3CB75000 00000000: 00 40 80 00 00 00 00 00 00 00 00 00 00 00 00 00 > +[ 5.000000] ice 0x00000004 0x3CB75000 00000010: e0 00 00 00 00 00 04 00 > +[ 5.000000] ice 0000:01:00.0: AQ Response: opcode 0x0601, flags 0x3403, datalen 0x0018, retval 0x0000 > + cookie (h,l) 0x00000000 0x00000000 > + param (0,1) 0x00000000 0x00000000 > + addr (h,l) 0x00000004 0x3CB75000 > +[ 5.000000] ice 0x00000004 0x3CB75000 00000000: 00 40 80 00 00 00 00 00 00 00 00 00 00 00 00 00 > +[ 5.000000] ice 0x00000004 0x3CB75000 00000010: e0 00 00 00 00 00 04 00 I think we can append to samples/$platform.log for now. In the long run, I think having the same directory tree structure under samples/ as in what/. As in: 0x0601.log would be in samples/ice/opcodes/; but there is very little stuff available now, plus this was before I started tinkering with multiple platforms. Append it to samples/linux.log for now. The gist is we want $ make test to smoke everything that's available. > diff --git a/what/ice/opcodes/0x0601.lua b/what/ice/opcodes/0x0601.lua > new file mode 100644 > index 000000000000..e53c51005617 > --- /dev/null > +++ b/what/ice/opcodes/0x0601.lua > @@ -0,0 +1,238 @@ > +local blobs = require "what.blobs" > +local bits = require "what.bits" > +local flags = require "what.ice.flags" > +local bit = bits.bit bit is unused. Remove it, please. > +local set_phy_config = {} > +local dump = [[ > +--- > +opcode: 0x0601 (Set PHY Config) > +kind: %s %s > +flags: %s > +retval: 0x%x > +cookie: 0x%08x 0x%08x > +%s...]] > + > + > +function set_phy_config:dump () > + return dump:format( > + self.queue, self.kind, > + flags.dump(self.flags), > + self.retval, > + self.cookie_h, self.cookie_l, > + self:dump_indirect_buffer()) > +end Yeah, comment for myself to reduce the code duplication opcodes boilerplate generates. > + > + > +--- 3.3.3.2.1 Extended PHY Capabilities 128-Bit Word Structure[E810] > +--- Table 3-90. 128-Bit Word Extended PHY Capabilities[E810] > +local extended_phy_capabilities = { > + -- Word[0]: > + "100BASE-TX", > + "100M-SGMII", ... > + "200G-AUI2-C2C", > + nil, > + -- Word[3]: every bit is Reserved > +} Can you move this to what.ice.extended_phy_capabilities and require it here and in 0x0607 in a separate patch. Can be in a series. > + ... > +--- Table 3-28. Set PHY Config Command Data Structure[E810] > +local indirect_buffer = blobs.new { > + -- TODO: Consider handling >8 bytes in one range definition. > + -- TODO: Consider supporting default value if lookup fails > + ["0.0-3.7"] = blobs.insert_if_empty("PHY Type", bit_list(extended_phy_capabilities, 32)), > + ["4.0-7.7"] = blobs.insert_if_empty("PHY Type", bit_list(extended_phy_capabilities, 32, 32)), > + ["8.0-11.7"] = blobs.insert_if_empty("PHY Type", bit_list(extended_phy_capabilities, 32, 64)), > + ["12.0-15.7"] = blobs.insert_if_empty("PHY Type", bit_list(extended_phy_capabilities, 32, 96)), This will not work as expected. We need something like: ["0.0-3.7"] = blobs.concat("PHY Type", bit_list(...)), That would concatenate lists together (or: "append" at the end of lists/strings). This does not need a resolution from your side, if you address comments above but skip this, I'll merge this and work on this myself since this could be tricky (as in: super Lua-specific). > + ["16.0-16.1"] = blobs.list "Pause Ability" { ... > + [1] = "Strict", > + }, > +} Note to self to cross-check this with datasheet in v2. Thanks again!