[{"id":2912292,"web_url":"http://patchwork.ozlabs.org/comment/2912292/","msgid":"","list_archive_url":null,"date":"2022-06-14T08:32:41","subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","submitter":{"id":82259,"url":"http://patchwork.ozlabs.org/api/people/82259/","name":"Damien Le Moal","email":"damien.lemoal@opensource.wdc.com"},"content":"On 6/10/22 17:17, Serge Semin wrote:\n> Currently not all of the Port-specific capabilities listed in the\n\ns/listed/are listed\n\n> PORT_CMD-enumeration. Let's extend that set with the Cold Presence\n> Detection and Mechanical Presence Switch attached to the Port flags [1] so\n> to closeup the set of the platform-specific port-capabilities flags. Note\n> these flags are supposed to be set by the platform firmware if there is\n> one. Alternatively as we are about to do they can be set by means of the\n> OF properties.\n> \n> While at it replace PORT_IRQ_DEV_ILCK with PORT_IRQ_DMPS and fix the\n> comment there. In accordance with [2] that IRQ flag is supposed to\n> indicate the state of the signal coming from the Mechanical Presence\n> Switch.\n> \n> [1] Serial ATA AHCI 1.3.1 Specification, p.27\n> [2] Serial ATA AHCI 1.3.1 Specification, p.24, p.88\n> \n> Signed-off-by: Serge Semin \n> Reviewed-by: Hannes Reinecke \n> \n> ---\n> \n> Changelog v4:\n> - Fix the DMPS macros name in the patch log. (@Sergei Shtylyov)\n> ---\n> drivers/ata/ahci.h | 7 ++++++-\n> 1 file changed, 6 insertions(+), 1 deletion(-)\n> \n> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h\n> index 7d834deefeb9..f501531bd1b3 100644\n> --- a/drivers/ata/ahci.h\n> +++ b/drivers/ata/ahci.h\n> @@ -138,7 +138,7 @@ enum {\n> \tPORT_IRQ_BAD_PMP\t= (1 << 23), /* incorrect port multiplier */\n> \n> \tPORT_IRQ_PHYRDY\t\t= (1 << 22), /* PhyRdy changed */\n> -\tPORT_IRQ_DEV_ILCK\t= (1 << 7), /* device interlock */\n> +\tPORT_IRQ_DMPS\t\t= (1 << 7), /* mechanical presence status */\n> \tPORT_IRQ_CONNECT\t= (1 << 6), /* port connect change status */\n> \tPORT_IRQ_SG_DONE\t= (1 << 5), /* descriptor processed */\n> \tPORT_IRQ_UNK_FIS\t= (1 << 4), /* unknown FIS rx'd */\n> @@ -166,6 +166,8 @@ enum {\n> \tPORT_CMD_ATAPI\t\t= (1 << 24), /* Device is ATAPI */\n> \tPORT_CMD_FBSCP\t\t= (1 << 22), /* FBS Capable Port */\n> \tPORT_CMD_ESP\t\t= (1 << 21), /* External Sata Port */\n> +\tPORT_CMD_CPD\t\t= (1 << 20), /* Cold Presence Detection */\n> +\tPORT_CMD_MPSP\t\t= (1 << 19), /* Mechanical Presence Switch */\n> \tPORT_CMD_HPCP\t\t= (1 << 18), /* HotPlug Capable Port */\n> \tPORT_CMD_PMP\t\t= (1 << 17), /* PMP attached */\n> \tPORT_CMD_LIST_ON\t= (1 << 15), /* cmd list DMA engine running */\n> @@ -181,6 +183,9 @@ enum {\n> \tPORT_CMD_ICC_PARTIAL\t= (0x2 << 28), /* Put i/f in partial state */\n> \tPORT_CMD_ICC_SLUMBER\t= (0x6 << 28), /* Put i/f in slumber state */\n> \n> +\tPORT_CMD_CAP\t\t= PORT_CMD_HPCP | PORT_CMD_MPSP |\n> +\t\t\t\t PORT_CMD_CPD | PORT_CMD_ESP | PORT_CMD_FBSCP,\n\nWhat is this one for ? A comment above it would be nice.\n\n> +\n> \t/* PORT_FBS bits */\n> \tPORT_FBS_DWE_OFFSET\t= 16, /* FBS device with error offset */\n> \tPORT_FBS_ADO_OFFSET\t= 12, /* FBS active dev optimization offset */","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["bilbo.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n unprotected) header.d=wdc.com header.i=@wdc.com header.a=rsa-sha256\n header.s=dkim.wdc.com header.b=hrPvOR8q;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=opensource.wdc.com header.i=@opensource.wdc.com\n header.a=rsa-sha256 header.s=dkim header.b=k3YeNMEz;\n\tdkim-atps=neutral","ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2620:137:e000::1:20; helo=out1.vger.email;\n envelope-from=linux-ide-owner@vger.kernel.org; receiver=)","usg-ed-osssrv.wdc.com (amavisd-new); dkim=pass\n reason=\"pass (just generated, assumed good)\"\n header.d=opensource.wdc.com"],"Received":["from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20])\n\tby bilbo.ozlabs.org (Postfix) with ESMTP id 4LMhX42mQDz9sGh\n\tfor ; Tue, 14 Jun 2022 18:32:52 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S232338AbiFNIct (ORCPT );\n Tue, 14 Jun 2022 04:32:49 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:56926 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S231139AbiFNIcr (ORCPT\n ); Tue, 14 Jun 2022 04:32:47 -0400","from esa5.hgst.iphmx.com (esa5.hgst.iphmx.com [216.71.153.144])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 45933286E4\n for ; Tue, 14 Jun 2022 01:32:46 -0700 (PDT)","from h199-255-45-15.hgst.com (HELO uls-op-cesaep02.wdc.com)\n ([199.255.45.15])\n by ob1.hgst.iphmx.com with ESMTP; 14 Jun 2022 16:32:45 +0800","from uls-op-cesaip02.wdc.com ([10.248.3.37])\n by uls-op-cesaep02.wdc.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256;\n 14 Jun 2022 00:51:20 -0700","from usg-ed-osssrv.wdc.com ([10.3.10.180])\n by uls-op-cesaip02.wdc.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256;\n 14 Jun 2022 01:32:46 -0700","from usg-ed-osssrv.wdc.com (usg-ed-osssrv.wdc.com [127.0.0.1])\n by usg-ed-osssrv.wdc.com (Postfix) with ESMTP id 4LMhWx37f5z1SVp1\n for ; Tue, 14 Jun 2022 01:32:45 -0700 (PDT)","from usg-ed-osssrv.wdc.com ([127.0.0.1])\n by usg-ed-osssrv.wdc.com (usg-ed-osssrv.wdc.com [127.0.0.1])\n (amavisd-new, port 10026)\n with ESMTP id 9Fd2tWAsxChR for ;\n Tue, 14 Jun 2022 01:32:44 -0700 (PDT)","from [10.225.163.77] (unknown [10.225.163.77])\n by usg-ed-osssrv.wdc.com (Postfix) with ESMTPSA id 4LMhWt3sbCz1Rvlc;\n Tue, 14 Jun 2022 01:32:42 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=simple/simple;\n d=wdc.com; i=@wdc.com; q=dns/txt; s=dkim.wdc.com;\n t=1655195565; x=1686731565;\n h=message-id:date:mime-version:subject:to:cc:references:\n from:in-reply-to:content-transfer-encoding;\n bh=tjdGPEcRSdnZLqy6B+WSAxdgi3miYFwPZpstyD652UY=;\n b=hrPvOR8qkjdkM32wZF6cRuQRTnGTkI4p8K4yR7gAeRch7JwaLolTmsUv\n P/9uCL+HcpvPA/e/TmbYb/WOfUXJRs8EnkBFdM/I2RxivXNZcu3dBL5aX\n O5geb184wL2jRpEtvisSH49eT/tbdPhD8zFiJ29BjfWC6MATK1pZdaTpr\n +G8rMBL7LCG5YsObrj/L1+ddpde/Gky7mkHss8KhjLnX+G+oI8QUs0q9+\n G0LjbKTKj1xwYlzF2t7MVwu49fgZRmfROgoOszDpcnwlSNO5goVj5h0s+\n TYlR6i6dPoUJbvVOunXE7CK2yqGGlVOBaMAIcpl8kaJGUfOXrfWDmHdql\n w==;","v=1; a=rsa-sha256; c=relaxed/simple; d=\n opensource.wdc.com; h=content-transfer-encoding:content-type\n :in-reply-to:organization:from:references:to:content-language\n :subject:user-agent:mime-version:date:message-id; s=dkim; t=\n 1655195564; x=1657787565; bh=tjdGPEcRSdnZLqy6B+WSAxdgi3miYFwPZps\n tyD652UY=; b=k3YeNMEzcnW65cYyTPPM9eAGBEg7SXhxbXDThNf4HAGhgd+sD6e\n zvGdIfOfKD8+IzjHRj/vKCbBh1Fy9/blMBwLTgV3O7jl5QfrCxTweBH3RwD3881r\n FmnbClQutIzStBiD03C9OyAbby76lyZs7aG4NxM/BQXrq7Ax+abZ5Mkr0LowHRy2\n 9oO335fBLKTXdqCuJrW24iGALrMRysguswwig9UgnjPSKb+uEyDwNG85o9ewxzwV\n ti8pvNGZLZuG1uirZ+NN5SAAyX0zShyzf1l/HWSJp50maRGCh6Bb7L2UgMu+jRHS\n XiyuNCgppN/f5MKX94Ij/mOK5l18xxc68Pg=="],"X-IronPort-AV":"E=Sophos;i=\"5.91,299,1647273600\";\n d=\"scan'208\";a=\"203097365\"","IronPort-SDR":["\n aGdhnKB7/a0WRcCbdbQIoGzKL9tRyYJxpcNVMcTp00Dwef6u06uTr/DcdgfVun3i21l9IABP0e\n s2IwEaUxTKAsbqhh7ah6CvNIzWcL0m5RDY6EmEvCSqFeRWldDYpdQPU/h4bnMgBoaZumatgRZp\n m/NHaFmdCWxjZlL8Ov+JWmF+pPWA2T/IQ0MZmlF6bW/p/79JXLTecwQiWf7QIA/EMluX/NZe7p\n 4P184nJYiUFcftI9zm6h4T2+yW7g15NvkxIrRDOdb2adKcuWSBQpYhhgseadLk+oTnSf+PtJ0C\n eS2M+cm5KmJalNAj59CyJ+th","\n M0RqBDesqnFIejIfepD7Q9JXp32hRU472NGBfEIVpd+FVFJ5dbZ3kZ4JzWTE2aj1gOe31SOrry\n 7gXDD+CFoIxYq/bd4APgXp0WwiRp94W1ljG3W1VNQVrKCYOwuk8Tbo9OsnVzmoFqnfFZh/ON45\n z/dzEDsrmEzQc1ga6DDwCUzuXSbAHj/WDdic2mbEDzKD0f3Ol48ih1r4c/3zNDlM1Xw2j4/v18\n /j+g9NDmwNghp46Xbl0K8OFaF1EiyUNO3NrfwYWxBH3D3nrrfmfFp3BtXoi7DXwhelf+1Q6Txy\n g/0="],"WDCIronportException":"Internal","X-Virus-Scanned":"amavisd-new at usg-ed-osssrv.wdc.com","Message-ID":"","Date":"Tue, 14 Jun 2022 17:32:41 +0900","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n Thunderbird/91.10.0","Subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","Content-Language":"en-US","To":"Serge Semin ,\n Hans de Goede ,\n Jens Axboe , Hannes Reinecke ","Cc":"Serge Semin ,\n Alexey Malahov ,\n Pavel Parkhomenko ,\n Rob Herring , linux-ide@vger.kernel.org,\n linux-kernel@vger.kernel.org, devicetree@vger.kernel.org","References":"<20220610081801.11854-1-Sergey.Semin@baikalelectronics.ru>\n <20220610081801.11854-13-Sergey.Semin@baikalelectronics.ru>","From":"Damien Le Moal ","Organization":"Western Digital Research","In-Reply-To":"<20220610081801.11854-13-Sergey.Semin@baikalelectronics.ru>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","X-Spam-Status":"No, score=-5.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,\n DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_MED,\n SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham\n autolearn_force=no version=3.4.6","X-Spam-Checker-Version":"SpamAssassin 3.4.6 (2021-04-09) on\n lindbergh.monkeyblade.net","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ide@vger.kernel.org"}},{"id":2913722,"web_url":"http://patchwork.ozlabs.org/comment/2913722/","msgid":"<20220615205819.uiqptkqm5qfdvrbj@mobilestation>","list_archive_url":null,"date":"2022-06-15T20:58:19","subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","submitter":{"id":70038,"url":"http://patchwork.ozlabs.org/api/people/70038/","name":"Serge Semin","email":"fancer.lancer@gmail.com"},"content":"On Tue, Jun 14, 2022 at 05:32:41PM +0900, Damien Le Moal wrote:\n> On 6/10/22 17:17, Serge Semin wrote:\n> > Currently not all of the Port-specific capabilities listed in the\n> \n> s/listed/are listed\n> \n> > PORT_CMD-enumeration. Let's extend that set with the Cold Presence\n> > Detection and Mechanical Presence Switch attached to the Port flags [1] so\n> > to closeup the set of the platform-specific port-capabilities flags. Note\n> > these flags are supposed to be set by the platform firmware if there is\n> > one. Alternatively as we are about to do they can be set by means of the\n> > OF properties.\n> > \n> > While at it replace PORT_IRQ_DEV_ILCK with PORT_IRQ_DMPS and fix the\n> > comment there. In accordance with [2] that IRQ flag is supposed to\n> > indicate the state of the signal coming from the Mechanical Presence\n> > Switch.\n> > \n> > [1] Serial ATA AHCI 1.3.1 Specification, p.27\n> > [2] Serial ATA AHCI 1.3.1 Specification, p.24, p.88\n> > \n> > Signed-off-by: Serge Semin \n> > Reviewed-by: Hannes Reinecke \n> > \n> > ---\n> > \n> > Changelog v4:\n> > - Fix the DMPS macros name in the patch log. (@Sergei Shtylyov)\n> > ---\n> > drivers/ata/ahci.h | 7 ++++++-\n> > 1 file changed, 6 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h\n> > index 7d834deefeb9..f501531bd1b3 100644\n> > --- a/drivers/ata/ahci.h\n> > +++ b/drivers/ata/ahci.h\n> > @@ -138,7 +138,7 @@ enum {\n> > \tPORT_IRQ_BAD_PMP\t= (1 << 23), /* incorrect port multiplier */\n> > \n> > \tPORT_IRQ_PHYRDY\t\t= (1 << 22), /* PhyRdy changed */\n> > -\tPORT_IRQ_DEV_ILCK\t= (1 << 7), /* device interlock */\n> > +\tPORT_IRQ_DMPS\t\t= (1 << 7), /* mechanical presence status */\n> > \tPORT_IRQ_CONNECT\t= (1 << 6), /* port connect change status */\n> > \tPORT_IRQ_SG_DONE\t= (1 << 5), /* descriptor processed */\n> > \tPORT_IRQ_UNK_FIS\t= (1 << 4), /* unknown FIS rx'd */\n> > @@ -166,6 +166,8 @@ enum {\n> > \tPORT_CMD_ATAPI\t\t= (1 << 24), /* Device is ATAPI */\n> > \tPORT_CMD_FBSCP\t\t= (1 << 22), /* FBS Capable Port */\n> > \tPORT_CMD_ESP\t\t= (1 << 21), /* External Sata Port */\n> > +\tPORT_CMD_CPD\t\t= (1 << 20), /* Cold Presence Detection */\n> > +\tPORT_CMD_MPSP\t\t= (1 << 19), /* Mechanical Presence Switch */\n> > \tPORT_CMD_HPCP\t\t= (1 << 18), /* HotPlug Capable Port */\n> > \tPORT_CMD_PMP\t\t= (1 << 17), /* PMP attached */\n> > \tPORT_CMD_LIST_ON\t= (1 << 15), /* cmd list DMA engine running */\n> > @@ -181,6 +183,9 @@ enum {\n> > \tPORT_CMD_ICC_PARTIAL\t= (0x2 << 28), /* Put i/f in partial state */\n> > \tPORT_CMD_ICC_SLUMBER\t= (0x6 << 28), /* Put i/f in slumber state */\n> > \n> > +\tPORT_CMD_CAP\t\t= PORT_CMD_HPCP | PORT_CMD_MPSP |\n> > +\t\t\t\t PORT_CMD_CPD | PORT_CMD_ESP | PORT_CMD_FBSCP,\n> \n\n> What is this one for ? A comment above it would be nice.\n\nIsn't it obviously inferrable from the definition and the item name?\n\n-Sergey\n\n> \n> > +\n> > \t/* PORT_FBS bits */\n> > \tPORT_FBS_DWE_OFFSET\t= 16, /* FBS device with error offset */\n> > \tPORT_FBS_ADO_OFFSET\t= 12, /* FBS active dev optimization offset */\n> \n> \n> -- \n> Damien Le Moal\n> Western Digital Research","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["bilbo.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256\n header.s=20210112 header.b=fw27TONp;\n\tdkim-atps=neutral","ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2620:137:e000::1:20; helo=out1.vger.email;\n envelope-from=linux-ide-owner@vger.kernel.org; receiver=)"],"Received":["from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20])\n\tby bilbo.ozlabs.org (Postfix) with ESMTP id 4LNd2Q6v1bz9sFx\n\tfor ; Thu, 16 Jun 2022 06:58:54 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S1345717AbiFOU6w (ORCPT );\n Wed, 15 Jun 2022 16:58:52 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:53232 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S1349740AbiFOU6Z (ORCPT\n ); Wed, 15 Jun 2022 16:58:25 -0400","from mail-lf1-x12a.google.com (mail-lf1-x12a.google.com\n [IPv6:2a00:1450:4864:20::12a])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D783EE01C;\n Wed, 15 Jun 2022 13:58:23 -0700 (PDT)","by mail-lf1-x12a.google.com with SMTP id w20so20719741lfa.11;\n Wed, 15 Jun 2022 13:58:23 -0700 (PDT)","from mobilestation ([95.79.189.214])\n by smtp.gmail.com with ESMTPSA id\n a11-20020a2eb16b000000b00253d4db8a92sm1743441ljm.63.2022.06.15.13.58.20\n (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n Wed, 15 Jun 2022 13:58:21 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=gmail.com; s=20210112;\n h=date:from:to:cc:subject:message-id:references:mime-version\n :content-disposition:in-reply-to;\n bh=YHkVcZcapgrq+x05D7RsWPsdaTufeMbFq41xO3naPPo=;\n b=fw27TONpdNc68v0SIqqXR74uavoS0smeRRHwI/m7lMWREdY+KTy4GA3YFBoo9QSIqw\n oOOuGS9VWVmUa/lj/HFdJsPsA5tDkr0fvdIGRaMJP97qw3jeE0GJ8IUKNomxEexcoENH\n BeLnJJ95rc5BkT+NsMcqLUdxLX9Tk+FSl8xk/kqdrR4D97M8BVtpr0Kv+uSLIKWu1jRf\n h3c0U/bA0LVsZPARWudbPkeHqcCwwg0ybAvj4Z9UwIz+wyc/6/RSpW4gIk/TgBq3Kbuf\n TEsm2/k0DflGOVvbWMIk3FfrZqAheovTj9v0PFO0lim/CrjjmmZ/N4Y464SH6eIYayKm\n j1zQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20210112;\n h=x-gm-message-state:date:from:to:cc:subject:message-id:references\n :mime-version:content-disposition:in-reply-to;\n bh=YHkVcZcapgrq+x05D7RsWPsdaTufeMbFq41xO3naPPo=;\n b=H7FHOmMtur0dukCxyuTI6tGrCFuESU0h/etMvXBWVRUuqanElm17SJE19rOtJ+JX58\n E0ExxueflW0WtUtv7/ooYA8ItHA57Ljyc/oMO4rQxYJYfVioR1Po1zmlK/s1WMQxGPzb\n nYZxZtDns/IyBmka9ytJjBLoMCqHGTuvKo7xv5m63T14seIUX3ELO3SQHS7Xh+IXusF7\n LR0140ncW/oL9POuWKUm/2pHhD6iiQm6IZ/uVRr9Z8nGzly/q8xfPCc3EO0I+gmQh0gA\n xre7BMT/CuVNFEvggeZ/CA25Oukz0C91hBD6WzLUmwOYvMeaLchJT+sQChEO35gYjZya\n EQnA==","X-Gm-Message-State":"AJIora9JG51jw7pBQK/1GGFivU5KRtCIOf7bgvb79QkKaXFVfkA7TYwX\n MbHKc7zpza62fhmGH34v3sg=","X-Google-Smtp-Source":"\n AGRyM1vTVX7Wn1vQHuKb0FjphhadAW3sNTj+3aQQu2P/WhWKfnU42vseAltEUbX1dYROiudbz0X9Jg==","X-Received":"by 2002:a05:6512:68e:b0:47d:ae64:fcf4 with SMTP id\n t14-20020a056512068e00b0047dae64fcf4mr747871lfe.685.1655326702146;\n Wed, 15 Jun 2022 13:58:22 -0700 (PDT)","Date":"Wed, 15 Jun 2022 23:58:19 +0300","From":"Serge Semin ","To":"Damien Le Moal ","Cc":"Serge Semin ,\n Hans de Goede ,\n Jens Axboe , Hannes Reinecke ,\n Alexey Malahov ,\n Pavel Parkhomenko ,\n Rob Herring , linux-ide@vger.kernel.org,\n linux-kernel@vger.kernel.org, devicetree@vger.kernel.org","Subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","Message-ID":"<20220615205819.uiqptkqm5qfdvrbj@mobilestation>","References":"<20220610081801.11854-1-Sergey.Semin@baikalelectronics.ru>\n <20220610081801.11854-13-Sergey.Semin@baikalelectronics.ru>\n ","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","X-Spam-Status":"No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,\n DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,\n RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE\n autolearn=ham autolearn_force=no version=3.4.6","X-Spam-Checker-Version":"SpamAssassin 3.4.6 (2021-04-09) on\n lindbergh.monkeyblade.net","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ide@vger.kernel.org"}},{"id":2913853,"web_url":"http://patchwork.ozlabs.org/comment/2913853/","msgid":"<903e273a-9dc5-f0df-5391-e96e63318323@opensource.wdc.com>","list_archive_url":null,"date":"2022-06-16T00:28:18","subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","submitter":{"id":82259,"url":"http://patchwork.ozlabs.org/api/people/82259/","name":"Damien Le Moal","email":"damien.lemoal@opensource.wdc.com"},"content":"On 2022/06/16 5:58, Serge Semin wrote:\n> On Tue, Jun 14, 2022 at 05:32:41PM +0900, Damien Le Moal wrote:\n>> On 6/10/22 17:17, Serge Semin wrote:\n>>> Currently not all of the Port-specific capabilities listed in the\n>>\n>> s/listed/are listed\n>>\n>>> PORT_CMD-enumeration. Let's extend that set with the Cold Presence\n>>> Detection and Mechanical Presence Switch attached to the Port flags [1] so\n>>> to closeup the set of the platform-specific port-capabilities flags. Note\n>>> these flags are supposed to be set by the platform firmware if there is\n>>> one. Alternatively as we are about to do they can be set by means of the\n>>> OF properties.\n>>>\n>>> While at it replace PORT_IRQ_DEV_ILCK with PORT_IRQ_DMPS and fix the\n>>> comment there. In accordance with [2] that IRQ flag is supposed to\n>>> indicate the state of the signal coming from the Mechanical Presence\n>>> Switch.\n>>>\n>>> [1] Serial ATA AHCI 1.3.1 Specification, p.27\n>>> [2] Serial ATA AHCI 1.3.1 Specification, p.24, p.88\n>>>\n>>> Signed-off-by: Serge Semin \n>>> Reviewed-by: Hannes Reinecke \n>>>\n>>> ---\n>>>\n>>> Changelog v4:\n>>> - Fix the DMPS macros name in the patch log. (@Sergei Shtylyov)\n>>> ---\n>>> drivers/ata/ahci.h | 7 ++++++-\n>>> 1 file changed, 6 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h\n>>> index 7d834deefeb9..f501531bd1b3 100644\n>>> --- a/drivers/ata/ahci.h\n>>> +++ b/drivers/ata/ahci.h\n>>> @@ -138,7 +138,7 @@ enum {\n>>> \tPORT_IRQ_BAD_PMP\t= (1 << 23), /* incorrect port multiplier */\n>>> \n>>> \tPORT_IRQ_PHYRDY\t\t= (1 << 22), /* PhyRdy changed */\n>>> -\tPORT_IRQ_DEV_ILCK\t= (1 << 7), /* device interlock */\n>>> +\tPORT_IRQ_DMPS\t\t= (1 << 7), /* mechanical presence status */\n>>> \tPORT_IRQ_CONNECT\t= (1 << 6), /* port connect change status */\n>>> \tPORT_IRQ_SG_DONE\t= (1 << 5), /* descriptor processed */\n>>> \tPORT_IRQ_UNK_FIS\t= (1 << 4), /* unknown FIS rx'd */\n>>> @@ -166,6 +166,8 @@ enum {\n>>> \tPORT_CMD_ATAPI\t\t= (1 << 24), /* Device is ATAPI */\n>>> \tPORT_CMD_FBSCP\t\t= (1 << 22), /* FBS Capable Port */\n>>> \tPORT_CMD_ESP\t\t= (1 << 21), /* External Sata Port */\n>>> +\tPORT_CMD_CPD\t\t= (1 << 20), /* Cold Presence Detection */\n>>> +\tPORT_CMD_MPSP\t\t= (1 << 19), /* Mechanical Presence Switch */\n>>> \tPORT_CMD_HPCP\t\t= (1 << 18), /* HotPlug Capable Port */\n>>> \tPORT_CMD_PMP\t\t= (1 << 17), /* PMP attached */\n>>> \tPORT_CMD_LIST_ON\t= (1 << 15), /* cmd list DMA engine running */\n>>> @@ -181,6 +183,9 @@ enum {\n>>> \tPORT_CMD_ICC_PARTIAL\t= (0x2 << 28), /* Put i/f in partial state */\n>>> \tPORT_CMD_ICC_SLUMBER\t= (0x6 << 28), /* Put i/f in slumber state */\n>>> \n>>> +\tPORT_CMD_CAP\t\t= PORT_CMD_HPCP | PORT_CMD_MPSP |\n>>> +\t\t\t\t PORT_CMD_CPD | PORT_CMD_ESP | PORT_CMD_FBSCP,\n>>\n> \n>> What is this one for ? A comment above it would be nice.\n> \n> Isn't it obviously inferrable from the definition and the item name?\n\nI am guessing from the name. Am I guessing OK ? A comment would still be nice.\nWhy just these bits ? There are more cap/support indicator bits in that port cmd\nbitfield. So why this particular set of bits ? What do they mean all together ?\n\nSure I can go and read the specs to figure it out. But again, a comment would\navoid readers of the code to have to decrypt all that.\n\n> \n> -Sergey\n> \n>>\n>>> +\n>>> \t/* PORT_FBS bits */\n>>> \tPORT_FBS_DWE_OFFSET\t= 16, /* FBS device with error offset */\n>>> \tPORT_FBS_ADO_OFFSET\t= 12, /* FBS active dev optimization offset */\n>>\n>>\n>> -- \n>> Damien Le Moal\n>> Western Digital Research","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["bilbo.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n unprotected) header.d=wdc.com header.i=@wdc.com header.a=rsa-sha256\n header.s=dkim.wdc.com header.b=M9EOGZZH;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=opensource.wdc.com header.i=@opensource.wdc.com\n header.a=rsa-sha256 header.s=dkim header.b=f1mDXGtb;\n\tdkim-atps=neutral","ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2620:137:e000::1:20; helo=out1.vger.email;\n envelope-from=linux-ide-owner@vger.kernel.org; receiver=)","usg-ed-osssrv.wdc.com (amavisd-new); dkim=pass\n reason=\"pass (just generated, assumed good)\"\n header.d=opensource.wdc.com"],"Received":["from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20])\n\tby bilbo.ozlabs.org (Postfix) with ESMTP id 4LNjjG2Bq6z9s2R\n\tfor ; Thu, 16 Jun 2022 10:29:22 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S243455AbiFPA3V (ORCPT );\n Wed, 15 Jun 2022 20:29:21 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:35972 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S1347709AbiFPA20 (ORCPT\n ); Wed, 15 Jun 2022 20:28:26 -0400","from esa6.hgst.iphmx.com (esa6.hgst.iphmx.com [216.71.154.45])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1160B12A98\n for ; Wed, 15 Jun 2022 17:28:25 -0700 (PDT)","from uls-op-cesaip02.wdc.com (HELO uls-op-cesaep02.wdc.com)\n ([199.255.45.15])\n by ob1.hgst.iphmx.com with ESMTP; 16 Jun 2022 08:28:24 +0800","from uls-op-cesaip01.wdc.com ([10.248.3.36])\n by uls-op-cesaep02.wdc.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256;\n 15 Jun 2022 16:46:46 -0700","from usg-ed-osssrv.wdc.com ([10.3.10.180])\n by uls-op-cesaip01.wdc.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256;\n 15 Jun 2022 17:28:23 -0700","from usg-ed-osssrv.wdc.com (usg-ed-osssrv.wdc.com [127.0.0.1])\n by usg-ed-osssrv.wdc.com (Postfix) with ESMTP id 4LNjh64Ch2z1Rwrw\n for ; Wed, 15 Jun 2022 17:28:22 -0700 (PDT)","from usg-ed-osssrv.wdc.com ([127.0.0.1])\n by usg-ed-osssrv.wdc.com (usg-ed-osssrv.wdc.com [127.0.0.1])\n (amavisd-new, port 10026)\n with ESMTP id UbiZ95XI9KB5 for ;\n Wed, 15 Jun 2022 17:28:21 -0700 (PDT)","from [10.89.84.185] (c02drav6md6t.dhcp.fujisawa.hgst.com\n [10.89.84.185])\n by usg-ed-osssrv.wdc.com (Postfix) with ESMTPSA id 4LNjh36YgPz1Rvlc;\n Wed, 15 Jun 2022 17:28:19 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=simple/simple;\n d=wdc.com; i=@wdc.com; q=dns/txt; s=dkim.wdc.com;\n t=1655339305; x=1686875305;\n h=message-id:date:mime-version:subject:to:cc:references:\n from:in-reply-to:content-transfer-encoding;\n bh=uYVFjpxrkNkhxj61fyi8GC6kWmJYxCqHfowX+smn4Q8=;\n b=M9EOGZZH5PodXB2uiAyRquTe20ZUaoDnbajM3s4mvUrsGLAsY+Hn3KsI\n 5DwtaqFd6/b2tVihScs0YXgsFF1FqtNvV1qcS4+m7Q8Ph1zkuIgzbzM7D\n 18jK6j+b+X0euiRJpRGcTx50ixa7MB9jv9hGblRXZI0tmdtWCOs9X6EAq\n IXW1dW611lTcT9OLvYp6Pu3QTtFEh3+zfkMM/pdiaV4Lx29LSzuNhCX3C\n 9EYGdQK9Mu8E9kq9rQsWb1eGMvwawUXuHSwwQQTzggM3VGMRaef/JA+hJ\n Sy0/QA3XHoAJgqH22PJUnnzaAnb2uc7mek9QV87E2lwESI0ptpf8zE2uX\n A==;","v=1; a=rsa-sha256; c=relaxed/simple; d=\n opensource.wdc.com; h=content-transfer-encoding:content-type\n :in-reply-to:organization:from:references:to:content-language\n :subject:user-agent:mime-version:date:message-id; s=dkim; t=\n 1655339301; x=1657931302; bh=uYVFjpxrkNkhxj61fyi8GC6kWmJYxCqHfow\n X+smn4Q8=; b=f1mDXGtbN3/m1SlybA62kaNKiCv2e1mA2QRjgr07kkClwUdJA9Z\n RVAlJ2Y3xnYcWSzvDEihYHI5nz/pGGOTrpGK1W2o4CmtdJwaJHjhMyeq/mzqWLG8\n 7QqQu2XjHIPMHsEFOpjS576QEvoPZYRPl4mHk3VgWXOP5Cn8s645F9Xckkf6DUDS\n tCDWMv8rFKHIt8qsJ8rh4Zlg/BE8jMpyFtBxQbXHrZ/7GoMZ9vvCvP4Kz8/8Ixgj\n aY4/20MVVqEnTUYkmTl62DJzFih7PVV48O8vt63j1/Vedr1EWgrcnTS+rC44PCAe\n 2d0ncZPolaZ9SqEkyl1BcacLpw0w6h3IToQ=="],"X-IronPort-AV":"E=Sophos;i=\"5.91,302,1647273600\";\n d=\"scan'208\";a=\"204038287\"","IronPort-SDR":["\n tQQfQh9/pPkzQTO4KmhtEii2t+UBDBHz6WpqbLmBG2zo5dGz9mzwQyfvm5diUDjoIw7BxifNTa\n jOApa2g59TYu8BTqJIx+bpFL2p5zryUSNcQIh0YrcDt3JXVogbeg0P+VifbK3AlmfD+M6jexWv\n IZj/3StB6kvCHDFznqIbB+2oh2pne+tIruDBrkgjpz8cUqmjlki4sGpusOpRy0grvWnISwX/DQ\n m3LMhhfuXNYV2qK+a4zs356A34TweqD/Qqy0t57pJMOmpk1kZUAiVHOTIfMZP3BnnExvUCg/Mk\n /zqesN0pZ/QR5TLK1wnW7Kxj","\n 5wqdbYTPm3Qw8F1Kf9fYT5AdP6oLbl/qfJAoxHvjeA9rLj/Qyi3pkjUUnrgBNh58QWFYu3qqJC\n b2uMALNvRUeV/8oQe2V2CsvONYlBpra1JS+aJGtxpLTQYUQAr4QUDF/dqwzANWw6ZuOH7PdNOn\n zYsnnC71NRGsXDx7B7T9jQosxHXyyZslrL4Tftrmhh2y85+mGl0krih7krIVA7S77B63I/WZeB\n VYxAM2Ygn3kxNAYR/n9CZJRlTGYAuto7fGO9hu22q5dbGb8RdAAmvwpTT15SgLbi4TatagOlnw\n nKs="],"WDCIronportException":"Internal","X-Virus-Scanned":"amavisd-new at usg-ed-osssrv.wdc.com","Message-ID":"<903e273a-9dc5-f0df-5391-e96e63318323@opensource.wdc.com>","Date":"Thu, 16 Jun 2022 09:28:18 +0900","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)\n Gecko/20100101 Thunderbird/91.10.0","Subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","Content-Language":"en-US","To":"Serge Semin ","Cc":"Serge Semin ,\n Hans de Goede ,\n Jens Axboe , Hannes Reinecke ,\n Alexey Malahov ,\n Pavel Parkhomenko ,\n Rob Herring , linux-ide@vger.kernel.org,\n linux-kernel@vger.kernel.org, devicetree@vger.kernel.org","References":"<20220610081801.11854-1-Sergey.Semin@baikalelectronics.ru>\n <20220610081801.11854-13-Sergey.Semin@baikalelectronics.ru>\n \n <20220615205819.uiqptkqm5qfdvrbj@mobilestation>","From":"Damien Le Moal ","Organization":"Western Digital Research","In-Reply-To":"<20220615205819.uiqptkqm5qfdvrbj@mobilestation>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","X-Spam-Status":"No, score=-5.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,\n DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_MED,\n SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable\n autolearn_force=no version=3.4.6","X-Spam-Checker-Version":"SpamAssassin 3.4.6 (2021-04-09) on\n lindbergh.monkeyblade.net","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ide@vger.kernel.org"}},{"id":2915219,"web_url":"http://patchwork.ozlabs.org/comment/2915219/","msgid":"<20220617203100.jg2o7ponolaenf6r@mobilestation>","list_archive_url":null,"date":"2022-06-17T20:31:00","subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","submitter":{"id":70038,"url":"http://patchwork.ozlabs.org/api/people/70038/","name":"Serge Semin","email":"fancer.lancer@gmail.com"},"content":"On Thu, Jun 16, 2022 at 09:28:18AM +0900, Damien Le Moal wrote:\n> On 2022/06/16 5:58, Serge Semin wrote:\n> > On Tue, Jun 14, 2022 at 05:32:41PM +0900, Damien Le Moal wrote:\n> >> On 6/10/22 17:17, Serge Semin wrote:\n> >>> Currently not all of the Port-specific capabilities listed in the\n> >>\n> >> s/listed/are listed\n> >>\n> >>> PORT_CMD-enumeration. Let's extend that set with the Cold Presence\n> >>> Detection and Mechanical Presence Switch attached to the Port flags [1] so\n> >>> to closeup the set of the platform-specific port-capabilities flags. Note\n> >>> these flags are supposed to be set by the platform firmware if there is\n> >>> one. Alternatively as we are about to do they can be set by means of the\n> >>> OF properties.\n> >>>\n> >>> While at it replace PORT_IRQ_DEV_ILCK with PORT_IRQ_DMPS and fix the\n> >>> comment there. In accordance with [2] that IRQ flag is supposed to\n> >>> indicate the state of the signal coming from the Mechanical Presence\n> >>> Switch.\n> >>>\n> >>> [1] Serial ATA AHCI 1.3.1 Specification, p.27\n> >>> [2] Serial ATA AHCI 1.3.1 Specification, p.24, p.88\n> >>>\n> >>> Signed-off-by: Serge Semin \n> >>> Reviewed-by: Hannes Reinecke \n> >>>\n> >>> ---\n> >>>\n> >>> Changelog v4:\n> >>> - Fix the DMPS macros name in the patch log. (@Sergei Shtylyov)\n> >>> ---\n> >>> drivers/ata/ahci.h | 7 ++++++-\n> >>> 1 file changed, 6 insertions(+), 1 deletion(-)\n> >>>\n> >>> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h\n> >>> index 7d834deefeb9..f501531bd1b3 100644\n> >>> --- a/drivers/ata/ahci.h\n> >>> +++ b/drivers/ata/ahci.h\n> >>> @@ -138,7 +138,7 @@ enum {\n> >>> \tPORT_IRQ_BAD_PMP\t= (1 << 23), /* incorrect port multiplier */\n> >>> \n> >>> \tPORT_IRQ_PHYRDY\t\t= (1 << 22), /* PhyRdy changed */\n> >>> -\tPORT_IRQ_DEV_ILCK\t= (1 << 7), /* device interlock */\n> >>> +\tPORT_IRQ_DMPS\t\t= (1 << 7), /* mechanical presence status */\n> >>> \tPORT_IRQ_CONNECT\t= (1 << 6), /* port connect change status */\n> >>> \tPORT_IRQ_SG_DONE\t= (1 << 5), /* descriptor processed */\n> >>> \tPORT_IRQ_UNK_FIS\t= (1 << 4), /* unknown FIS rx'd */\n> >>> @@ -166,6 +166,8 @@ enum {\n> >>> \tPORT_CMD_ATAPI\t\t= (1 << 24), /* Device is ATAPI */\n> >>> \tPORT_CMD_FBSCP\t\t= (1 << 22), /* FBS Capable Port */\n> >>> \tPORT_CMD_ESP\t\t= (1 << 21), /* External Sata Port */\n> >>> +\tPORT_CMD_CPD\t\t= (1 << 20), /* Cold Presence Detection */\n> >>> +\tPORT_CMD_MPSP\t\t= (1 << 19), /* Mechanical Presence Switch */\n> >>> \tPORT_CMD_HPCP\t\t= (1 << 18), /* HotPlug Capable Port */\n> >>> \tPORT_CMD_PMP\t\t= (1 << 17), /* PMP attached */\n> >>> \tPORT_CMD_LIST_ON\t= (1 << 15), /* cmd list DMA engine running */\n> >>> @@ -181,6 +183,9 @@ enum {\n> >>> \tPORT_CMD_ICC_PARTIAL\t= (0x2 << 28), /* Put i/f in partial state */\n> >>> \tPORT_CMD_ICC_SLUMBER\t= (0x6 << 28), /* Put i/f in slumber state */\n> >>> \n> >>> +\tPORT_CMD_CAP\t\t= PORT_CMD_HPCP | PORT_CMD_MPSP |\n> >>> +\t\t\t\t PORT_CMD_CPD | PORT_CMD_ESP | PORT_CMD_FBSCP,\n> >>\n> > \n> >> What is this one for ? A comment above it would be nice.\n> > \n> > Isn't it obviously inferrable from the definition and the item name?\n> \n\n> I am guessing from the name. Am I guessing OK ? A comment would still be nice.\n> Why just these bits ? There are more cap/support indicator bits in that port cmd\n> bitfield. So why this particular set of bits ? What do they mean all together ?\n\nNormally the variable/constant name should be self-content (as the\nkernel coding style doc states and what the common sense suggests). So\nthe reader could correctly guess its purpose/content/value. In this\ncase PORT_CMD_CAP - means PORT CMD capabilities mask. All of the\npossible flags have been set in that mask. There are no more\ncapabilities in the PORT CMD register left undeclared. That's why the\nname is selected the way it is and why I haven't added any comment in\nhere (what the kernel coding style says about the over-commenting the\ncode).\n\n> \n> Sure I can go and read the specs to figure it out. But again, a comment would\n> avoid readers of the code to have to decrypt all that.\n\nIf you still insist on having an additional comment. I can add\nsomething like \"/* PORT_CMD capabilities mask */\". Are you ok with it?\n\n-Sergey\n\n> \n> > \n> > -Sergey\n> > \n> >>\n> >>> +\n> >>> \t/* PORT_FBS bits */\n> >>> \tPORT_FBS_DWE_OFFSET\t= 16, /* FBS device with error offset */\n> >>> \tPORT_FBS_ADO_OFFSET\t= 12, /* FBS active dev optimization offset */\n> >>\n> >>\n> >> -- \n> >> Damien Le Moal\n> >> Western Digital Research\n> \n> \n> -- \n> Damien Le Moal\n> Western Digital Research","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["bilbo.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256\n header.s=20210112 header.b=V+CeIs+X;\n\tdkim-atps=neutral","ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2620:137:e000::1:20; helo=out1.vger.email;\n envelope-from=linux-ide-owner@vger.kernel.org; receiver=)"],"Received":["from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20])\n\tby bilbo.ozlabs.org (Postfix) with ESMTP id 4LPrKT0rp9z9sG0\n\tfor ; Sat, 18 Jun 2022 06:31:09 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S242751AbiFQUbI (ORCPT );\n Fri, 17 Jun 2022 16:31:08 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:60956 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S229952AbiFQUbH (ORCPT\n ); Fri, 17 Jun 2022 16:31:07 -0400","from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com\n [IPv6:2a00:1450:4864:20::22a])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E7D605C740;\n Fri, 17 Jun 2022 13:31:04 -0700 (PDT)","by mail-lj1-x22a.google.com with SMTP id h23so5871814ljl.3;\n Fri, 17 Jun 2022 13:31:04 -0700 (PDT)","from mobilestation ([95.79.189.214])\n by smtp.gmail.com with ESMTPSA id\n b9-20020a196449000000b00478ee191091sm752205lfj.153.2022.06.17.13.31.02\n (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n Fri, 17 Jun 2022 13:31:02 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=gmail.com; s=20210112;\n h=date:from:to:cc:subject:message-id:references:mime-version\n :content-disposition:in-reply-to;\n bh=7nGakZJxQXZKPcPy+VOb5gCeQUsCg6Cmp5BsLtz0ZEg=;\n b=V+CeIs+X7EbQen2649oLySuIcZZy/70d3iwfIO3HV8dvNrxh4huHI5BKcAZqjOyW39\n R+oVm+4XV5yC+B+hSoJ5WvRF6/dTv9Tva43rW1pGJc5L9sPGdwDFFbx1pHlzzjWQdkRn\n 8a4LBY3w3ZIb54U9W9xAh663OntWWyEqM4ubKTY5o8sdFiQsKSIrdza2NOnK2LvbbWmy\n YmStfXq9xRaxIvDAeMv+kRU810Duydzwfkfg/tkzF9Ay8AQ4aOvZYbezjccAlxzsf67H\n weXeI2jR3ibxarBv/RSEC+T/41WjeGvYKPr4PGKs/00MUTqeEibn3BMXda/L3udclTrb\n 1SBQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20210112;\n h=x-gm-message-state:date:from:to:cc:subject:message-id:references\n :mime-version:content-disposition:in-reply-to;\n bh=7nGakZJxQXZKPcPy+VOb5gCeQUsCg6Cmp5BsLtz0ZEg=;\n b=t5qwhy7ig1A99X8+Rj9CbEzJh/4IYhe54fD+GOw477otl/MrdVwtND4ziSq0pzcodK\n oPNTsR91VNLj8kFzFACxn9SgR/c0vTOySrgWyQTiLBpqN3GnSYV6F8hw+ldZbGZtF2UK\n dRXbzBOELn7o3n0cc1rKVmRH+7rUBkNM3qyqFAVK7IKx3byRUid/Jb/Yh/x9ufykQsme\n mo4WziwJCaHvbFEYEfqygB76kKSA3seoKUz0itZgZduMG7aK8T6YRKoVYQ+hFNKhEDzD\n NjdqpK92Nf9oMgr8TxVOLbyS/YwZYWt+g6uO9KJabF9etW+v3Y1OnY3CaQXsUwFYJ3T/\n pmcw==","X-Gm-Message-State":"AJIora/Zr6eDHEQazEFwHxAxDS4rcaf2wc7BYouowyFGK5ZmJthZwHWK\n +kT1SID+WlKwNrU9rqYwYuo=","X-Google-Smtp-Source":"\n AGRyM1u5enOCrssguLCkcYTXhPXubqKR3gfQ9XqPZS+m0o5UTd8X0N2UxqlADmuHWv28qOoGPeiS8w==","X-Received":"by 2002:a2e:9ece:0:b0:25a:4d0f:1540 with SMTP id\n h14-20020a2e9ece000000b0025a4d0f1540mr3893920ljk.481.1655497863284;\n Fri, 17 Jun 2022 13:31:03 -0700 (PDT)","Date":"Fri, 17 Jun 2022 23:31:00 +0300","From":"Serge Semin ","To":"Damien Le Moal ","Cc":"Serge Semin ,\n Hans de Goede ,\n Jens Axboe , Hannes Reinecke ,\n Alexey Malahov ,\n Pavel Parkhomenko ,\n Rob Herring , linux-ide@vger.kernel.org,\n linux-kernel@vger.kernel.org, devicetree@vger.kernel.org","Subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","Message-ID":"<20220617203100.jg2o7ponolaenf6r@mobilestation>","References":"<20220610081801.11854-1-Sergey.Semin@baikalelectronics.ru>\n <20220610081801.11854-13-Sergey.Semin@baikalelectronics.ru>\n \n <20220615205819.uiqptkqm5qfdvrbj@mobilestation>\n <903e273a-9dc5-f0df-5391-e96e63318323@opensource.wdc.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<903e273a-9dc5-f0df-5391-e96e63318323@opensource.wdc.com>","X-Spam-Status":"No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,\n DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,\n RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE\n autolearn=ham autolearn_force=no version=3.4.6","X-Spam-Checker-Version":"SpamAssassin 3.4.6 (2021-04-09) on\n lindbergh.monkeyblade.net","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ide@vger.kernel.org"}},{"id":2915383,"web_url":"http://patchwork.ozlabs.org/comment/2915383/","msgid":"","list_archive_url":null,"date":"2022-06-18T06:52:28","subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","submitter":{"id":82259,"url":"http://patchwork.ozlabs.org/api/people/82259/","name":"Damien Le Moal","email":"damien.lemoal@opensource.wdc.com"},"content":"On 6/18/22 05:31, Serge Semin wrote:\n> On Thu, Jun 16, 2022 at 09:28:18AM +0900, Damien Le Moal wrote:\n>> On 2022/06/16 5:58, Serge Semin wrote:\n>>> On Tue, Jun 14, 2022 at 05:32:41PM +0900, Damien Le Moal wrote:\n>>>> On 6/10/22 17:17, Serge Semin wrote:\n>>>>> Currently not all of the Port-specific capabilities listed in the\n>>>>\n>>>> s/listed/are listed\n>>>>\n>>>>> PORT_CMD-enumeration. Let's extend that set with the Cold Presence\n>>>>> Detection and Mechanical Presence Switch attached to the Port flags [1] so\n>>>>> to closeup the set of the platform-specific port-capabilities flags. Note\n>>>>> these flags are supposed to be set by the platform firmware if there is\n>>>>> one. Alternatively as we are about to do they can be set by means of the\n>>>>> OF properties.\n>>>>>\n>>>>> While at it replace PORT_IRQ_DEV_ILCK with PORT_IRQ_DMPS and fix the\n>>>>> comment there. In accordance with [2] that IRQ flag is supposed to\n>>>>> indicate the state of the signal coming from the Mechanical Presence\n>>>>> Switch.\n>>>>>\n>>>>> [1] Serial ATA AHCI 1.3.1 Specification, p.27\n>>>>> [2] Serial ATA AHCI 1.3.1 Specification, p.24, p.88\n>>>>>\n>>>>> Signed-off-by: Serge Semin \n>>>>> Reviewed-by: Hannes Reinecke \n>>>>>\n>>>>> ---\n>>>>>\n>>>>> Changelog v4:\n>>>>> - Fix the DMPS macros name in the patch log. (@Sergei Shtylyov)\n>>>>> ---\n>>>>> drivers/ata/ahci.h | 7 ++++++-\n>>>>> 1 file changed, 6 insertions(+), 1 deletion(-)\n>>>>>\n>>>>> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h\n>>>>> index 7d834deefeb9..f501531bd1b3 100644\n>>>>> --- a/drivers/ata/ahci.h\n>>>>> +++ b/drivers/ata/ahci.h\n>>>>> @@ -138,7 +138,7 @@ enum {\n>>>>> \tPORT_IRQ_BAD_PMP\t= (1 << 23), /* incorrect port multiplier */\n>>>>> \n>>>>> \tPORT_IRQ_PHYRDY\t\t= (1 << 22), /* PhyRdy changed */\n>>>>> -\tPORT_IRQ_DEV_ILCK\t= (1 << 7), /* device interlock */\n>>>>> +\tPORT_IRQ_DMPS\t\t= (1 << 7), /* mechanical presence status */\n>>>>> \tPORT_IRQ_CONNECT\t= (1 << 6), /* port connect change status */\n>>>>> \tPORT_IRQ_SG_DONE\t= (1 << 5), /* descriptor processed */\n>>>>> \tPORT_IRQ_UNK_FIS\t= (1 << 4), /* unknown FIS rx'd */\n>>>>> @@ -166,6 +166,8 @@ enum {\n>>>>> \tPORT_CMD_ATAPI\t\t= (1 << 24), /* Device is ATAPI */\n>>>>> \tPORT_CMD_FBSCP\t\t= (1 << 22), /* FBS Capable Port */\n>>>>> \tPORT_CMD_ESP\t\t= (1 << 21), /* External Sata Port */\n>>>>> +\tPORT_CMD_CPD\t\t= (1 << 20), /* Cold Presence Detection */\n>>>>> +\tPORT_CMD_MPSP\t\t= (1 << 19), /* Mechanical Presence Switch */\n>>>>> \tPORT_CMD_HPCP\t\t= (1 << 18), /* HotPlug Capable Port */\n>>>>> \tPORT_CMD_PMP\t\t= (1 << 17), /* PMP attached */\n>>>>> \tPORT_CMD_LIST_ON\t= (1 << 15), /* cmd list DMA engine running */\n>>>>> @@ -181,6 +183,9 @@ enum {\n>>>>> \tPORT_CMD_ICC_PARTIAL\t= (0x2 << 28), /* Put i/f in partial state */\n>>>>> \tPORT_CMD_ICC_SLUMBER\t= (0x6 << 28), /* Put i/f in slumber state */\n>>>>> \n>>>>> +\tPORT_CMD_CAP\t\t= PORT_CMD_HPCP | PORT_CMD_MPSP |\n>>>>> +\t\t\t\t PORT_CMD_CPD | PORT_CMD_ESP | PORT_CMD_FBSCP,\n>>>>\n>>>\n>>>> What is this one for ? A comment above it would be nice.\n>>>\n>>> Isn't it obviously inferrable from the definition and the item name?\n>>\n> \n>> I am guessing from the name. Am I guessing OK ? A comment would still be nice.\n>> Why just these bits ? There are more cap/support indicator bits in that port cmd\n>> bitfield. So why this particular set of bits ? What do they mean all together ?\n> \n> Normally the variable/constant name should be self-content (as the\n> kernel coding style doc states and what the common sense suggests). So\n> the reader could correctly guess its purpose/content/value. In this\n> case PORT_CMD_CAP - means PORT CMD capabilities mask. All of the\n> possible flags have been set in that mask. There are no more\n> capabilities in the PORT CMD register left undeclared. That's why the\n> name is selected the way it is and why I haven't added any comment in\n> here (what the kernel coding style says about the over-commenting the\n> code).\n\nYes, I understood from the name what it is. What I do NOT understand is\nwhy all the feature bits are not there. Why this subset only ? A comment\nabout that would be nice so that the reason for it is not lost.\n\n> \n>>\n>> Sure I can go and read the specs to figure it out. But again, a comment would\n>> avoid readers of the code to have to decrypt all that.\n> \n> If you still insist on having an additional comment. I can add\n> something like \"/* PORT_CMD capabilities mask */\". Are you ok with it?\n\nThat does not help on its own. The macro name says that already. I would\nlike a note about why only these features are selected.\n\n> \n> -Sergey\n> \n>>\n>>>\n>>> -Sergey\n>>>\n>>>>\n>>>>> +\n>>>>> \t/* PORT_FBS bits */\n>>>>> \tPORT_FBS_DWE_OFFSET\t= 16, /* FBS device with error offset */\n>>>>> \tPORT_FBS_ADO_OFFSET\t= 12, /* FBS active dev optimization offset */\n>>>>\n>>>>\n>>>> -- \n>>>> Damien Le Moal\n>>>> Western Digital Research\n>>\n>>\n>> -- \n>> Damien Le Moal\n>> Western Digital Research","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["bilbo.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n unprotected) header.d=wdc.com header.i=@wdc.com header.a=rsa-sha256\n header.s=dkim.wdc.com header.b=IeA4pbnP;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=opensource.wdc.com header.i=@opensource.wdc.com\n header.a=rsa-sha256 header.s=dkim header.b=TUpJ7+eb;\n\tdkim-atps=neutral","ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2620:137:e000::1:20; helo=out1.vger.email;\n envelope-from=linux-ide-owner@vger.kernel.org; receiver=)","usg-ed-osssrv.wdc.com (amavisd-new); dkim=pass\n reason=\"pass (just generated, assumed good)\"\n header.d=opensource.wdc.com"],"Received":["from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20])\n\tby bilbo.ozlabs.org (Postfix) with ESMTP id 4LQ66Z4cPPz9s75\n\tfor ; Sat, 18 Jun 2022 16:52:38 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S229760AbiFRGwh (ORCPT );\n Sat, 18 Jun 2022 02:52:37 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:56162 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S229478AbiFRGwf (ORCPT\n ); Sat, 18 Jun 2022 02:52:35 -0400","from esa6.hgst.iphmx.com (esa6.hgst.iphmx.com [216.71.154.45])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A795F48334\n for ; Fri, 17 Jun 2022 23:52:34 -0700 (PDT)","from uls-op-cesaip01.wdc.com (HELO uls-op-cesaep01.wdc.com)\n ([199.255.45.14])\n by ob1.hgst.iphmx.com with ESMTP; 18 Jun 2022 14:52:33 +0800","from uls-op-cesaip02.wdc.com ([10.248.3.37])\n by uls-op-cesaep01.wdc.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256;\n 17 Jun 2022 23:15:07 -0700","from usg-ed-osssrv.wdc.com ([10.3.10.180])\n by uls-op-cesaip02.wdc.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256;\n 17 Jun 2022 23:52:33 -0700","from usg-ed-osssrv.wdc.com (usg-ed-osssrv.wdc.com [127.0.0.1])\n by usg-ed-osssrv.wdc.com (Postfix) with ESMTP id 4LQ66S1cY8z1SHwl\n for ; Fri, 17 Jun 2022 23:52:32 -0700 (PDT)","from usg-ed-osssrv.wdc.com ([127.0.0.1])\n by usg-ed-osssrv.wdc.com (usg-ed-osssrv.wdc.com [127.0.0.1])\n (amavisd-new, port 10026)\n with ESMTP id 8hZn-WVVIk5y for ;\n Fri, 17 Jun 2022 23:52:31 -0700 (PDT)","from [10.225.163.84] (unknown [10.225.163.84])\n by usg-ed-osssrv.wdc.com (Postfix) with ESMTPSA id 4LQ66P39qWz1Rvlc;\n Fri, 17 Jun 2022 23:52:29 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=simple/simple;\n d=wdc.com; i=@wdc.com; q=dns/txt; s=dkim.wdc.com;\n t=1655535155; x=1687071155;\n h=message-id:date:mime-version:subject:to:cc:references:\n from:in-reply-to:content-transfer-encoding;\n bh=8uHQ9MiiAPuzcAYC2a1/X5+Fg5Ls+OxZmIChY7eTPTo=;\n b=IeA4pbnPdM9in5Sty31gl+g86qDCziajXz6L8wmgPLjz5sUl5BD7+eQM\n kDSxu8+Hroj8fRTKEGavIb17l79vQ4NAxbYg6wV6SrEWcz6vDMXXzppYG\n w6vrtD0iEvQ2M2W0ah5MUbkrJSvIQxeCnzGe4WYA72SHdO9YmclT/G/Pa\n g+Dm5YzLKk3NODQ1B3wE8DKSqwhvliyImVGJ/oB4MKiYhVeWcIWO7VX8/\n nxy9xoEFUevYufAD2CRElYwomzOv8xK074AOgtgYO63x7Zj+IFxhBKBh9\n /D6TJqJPADDArGIrFyI7GRu+JsYV81w+E0ViXsOXk3mK0wQYpEiPUA5BT\n g==;","v=1; a=rsa-sha256; c=relaxed/simple; d=\n opensource.wdc.com; h=content-transfer-encoding:content-type\n :in-reply-to:organization:from:references:to:content-language\n :subject:user-agent:mime-version:date:message-id; s=dkim; t=\n 1655535151; x=1658127152; bh=8uHQ9MiiAPuzcAYC2a1/X5+Fg5Ls+OxZmIC\n hY7eTPTo=; b=TUpJ7+ebXL/gL11xJaFgaSyqdV1HX0y+gJhPQYh1RWrl/kfOvXD\n X9ibxuxriilkY06CD6RvqsfDaU9baWmtRaLLutlL5Okvyhqqghh/il1Za4PYSzB2\n oLDbJKOjPcWcd+eM/vIsmTPugdeDJTQnWlyGSHmLNlL6kzBsfrHuWdPTdfI87BbE\n 8KxQc3vBGATGLW1RmJ+0tPNOuaSZ8dOeFfoXguxKbnKX0QJlAeBZzKTdqYInrQjR\n foSQI3moOeez0AhxAMYUnKTNdmxT8HvLOMYYhp2OAymPjrn5Ty6sm6ffs/kCwEnn\n ZJZ6ICx1nhiQcNLL8WXg7pGG2rqK47o+U5g=="],"X-IronPort-AV":"E=Sophos;i=\"5.92,306,1650902400\";\n d=\"scan'208\";a=\"204259540\"","IronPort-SDR":["\n KCKGam5jWA7s7v10mbalMpC2RKowNTI+acsffijuGF1g8vghq1O+yLShMLalZDeCmoNIiLIoMz\n dreqsCSNl9Y9fSawpxCgTJV1+rWKMtYjCxZKmLipdiEpOm99rPSCJAahThOc1fAVBaRJEbxRHH\n 3Hq9D9vMaQNchVQSdp4fpd17BIGsRunGCyxLGEDqPLsE88XouNAJ3IBnYH52b2e7Yohjd3GVIG\n q/AYaKRPP6/21lRvdwckE9kVqeNEKy8KBSW7hwVbgzi4amIlG5NBZpLVMFWojiRyHl4fTT6Jew\n bx5nUcMbGxTT1F+DcR0HJVnx","\n HBJRhhR5I3g9zLBTzT4QJichsrajNjBfwGpYX4bdGqVJpDI258i9vUJZTE4qiwTV3p2bZj1ovL\n 0K/KKTQaIZO9n1OpMxb+b6XEpbEZ9n3ayfdQKL+v6VFuzkSJQwOJeu1H3b4+ln5E/lmEjZE0qB\n JufZWnQOd75XCVj+imxM4hzsYXykQi1lmAuoqpkb2yEih/ZnXeRj/yoKc3cIOmy1lLSQ9WyOVE\n GadHcr4f1IXAU6xmShAl7ChAnktQiQB46lgVq3Q8Z/HBMgEpCg5rkpzh38umsdDHUEnxy7zwa8\n eRQ="],"WDCIronportException":"Internal","X-Virus-Scanned":"amavisd-new at usg-ed-osssrv.wdc.com","Message-ID":"","Date":"Sat, 18 Jun 2022 15:52:28 +0900","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n Thunderbird/91.10.0","Subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","Content-Language":"en-US","To":"Serge Semin ","Cc":"Serge Semin ,\n Hans de Goede ,\n Jens Axboe , Hannes Reinecke ,\n Alexey Malahov ,\n Pavel Parkhomenko ,\n Rob Herring , linux-ide@vger.kernel.org,\n linux-kernel@vger.kernel.org, devicetree@vger.kernel.org","References":"<20220610081801.11854-1-Sergey.Semin@baikalelectronics.ru>\n <20220610081801.11854-13-Sergey.Semin@baikalelectronics.ru>\n \n <20220615205819.uiqptkqm5qfdvrbj@mobilestation>\n <903e273a-9dc5-f0df-5391-e96e63318323@opensource.wdc.com>\n <20220617203100.jg2o7ponolaenf6r@mobilestation>","From":"Damien Le Moal ","Organization":"Western Digital Research","In-Reply-To":"<20220617203100.jg2o7ponolaenf6r@mobilestation>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","X-Spam-Status":"No, score=-6.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,\n DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_MED,\n SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable\n autolearn_force=no version=3.4.6","X-Spam-Checker-Version":"SpamAssassin 3.4.6 (2021-04-09) on\n lindbergh.monkeyblade.net","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ide@vger.kernel.org"}},{"id":2915390,"web_url":"http://patchwork.ozlabs.org/comment/2915390/","msgid":"<20220618081055.grsrjxa5gqiuhy2i@mobilestation>","list_archive_url":null,"date":"2022-06-18T08:10:55","subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","submitter":{"id":70038,"url":"http://patchwork.ozlabs.org/api/people/70038/","name":"Serge Semin","email":"fancer.lancer@gmail.com"},"content":"On Sat, Jun 18, 2022 at 03:52:28PM +0900, Damien Le Moal wrote:\n> On 6/18/22 05:31, Serge Semin wrote:\n> > On Thu, Jun 16, 2022 at 09:28:18AM +0900, Damien Le Moal wrote:\n> >> On 2022/06/16 5:58, Serge Semin wrote:\n> >>> On Tue, Jun 14, 2022 at 05:32:41PM +0900, Damien Le Moal wrote:\n> >>>> On 6/10/22 17:17, Serge Semin wrote:\n> >>>>> Currently not all of the Port-specific capabilities listed in the\n> >>>>\n> >>>> s/listed/are listed\n> >>>>\n> >>>>> PORT_CMD-enumeration. Let's extend that set with the Cold Presence\n> >>>>> Detection and Mechanical Presence Switch attached to the Port flags [1] so\n> >>>>> to closeup the set of the platform-specific port-capabilities flags. Note\n> >>>>> these flags are supposed to be set by the platform firmware if there is\n> >>>>> one. Alternatively as we are about to do they can be set by means of the\n> >>>>> OF properties.\n> >>>>>\n> >>>>> While at it replace PORT_IRQ_DEV_ILCK with PORT_IRQ_DMPS and fix the\n> >>>>> comment there. In accordance with [2] that IRQ flag is supposed to\n> >>>>> indicate the state of the signal coming from the Mechanical Presence\n> >>>>> Switch.\n> >>>>>\n> >>>>> [1] Serial ATA AHCI 1.3.1 Specification, p.27\n> >>>>> [2] Serial ATA AHCI 1.3.1 Specification, p.24, p.88\n> >>>>>\n> >>>>> Signed-off-by: Serge Semin \n> >>>>> Reviewed-by: Hannes Reinecke \n> >>>>>\n> >>>>> ---\n> >>>>>\n> >>>>> Changelog v4:\n> >>>>> - Fix the DMPS macros name in the patch log. (@Sergei Shtylyov)\n> >>>>> ---\n> >>>>> drivers/ata/ahci.h | 7 ++++++-\n> >>>>> 1 file changed, 6 insertions(+), 1 deletion(-)\n> >>>>>\n> >>>>> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h\n> >>>>> index 7d834deefeb9..f501531bd1b3 100644\n> >>>>> --- a/drivers/ata/ahci.h\n> >>>>> +++ b/drivers/ata/ahci.h\n> >>>>> @@ -138,7 +138,7 @@ enum {\n> >>>>> \tPORT_IRQ_BAD_PMP\t= (1 << 23), /* incorrect port multiplier */\n> >>>>> \n> >>>>> \tPORT_IRQ_PHYRDY\t\t= (1 << 22), /* PhyRdy changed */\n> >>>>> -\tPORT_IRQ_DEV_ILCK\t= (1 << 7), /* device interlock */\n> >>>>> +\tPORT_IRQ_DMPS\t\t= (1 << 7), /* mechanical presence status */\n> >>>>> \tPORT_IRQ_CONNECT\t= (1 << 6), /* port connect change status */\n> >>>>> \tPORT_IRQ_SG_DONE\t= (1 << 5), /* descriptor processed */\n> >>>>> \tPORT_IRQ_UNK_FIS\t= (1 << 4), /* unknown FIS rx'd */\n> >>>>> @@ -166,6 +166,8 @@ enum {\n> >>>>> \tPORT_CMD_ATAPI\t\t= (1 << 24), /* Device is ATAPI */\n> >>>>> \tPORT_CMD_FBSCP\t\t= (1 << 22), /* FBS Capable Port */\n> >>>>> \tPORT_CMD_ESP\t\t= (1 << 21), /* External Sata Port */\n> >>>>> +\tPORT_CMD_CPD\t\t= (1 << 20), /* Cold Presence Detection */\n> >>>>> +\tPORT_CMD_MPSP\t\t= (1 << 19), /* Mechanical Presence Switch */\n> >>>>> \tPORT_CMD_HPCP\t\t= (1 << 18), /* HotPlug Capable Port */\n> >>>>> \tPORT_CMD_PMP\t\t= (1 << 17), /* PMP attached */\n> >>>>> \tPORT_CMD_LIST_ON\t= (1 << 15), /* cmd list DMA engine running */\n> >>>>> @@ -181,6 +183,9 @@ enum {\n> >>>>> \tPORT_CMD_ICC_PARTIAL\t= (0x2 << 28), /* Put i/f in partial state */\n> >>>>> \tPORT_CMD_ICC_SLUMBER\t= (0x6 << 28), /* Put i/f in slumber state */\n> >>>>> \n> >>>>> +\tPORT_CMD_CAP\t\t= PORT_CMD_HPCP | PORT_CMD_MPSP |\n> >>>>> +\t\t\t\t PORT_CMD_CPD | PORT_CMD_ESP | PORT_CMD_FBSCP,\n> >>>>\n> >>>\n> >>>> What is this one for ? A comment above it would be nice.\n> >>>\n> >>> Isn't it obviously inferrable from the definition and the item name?\n> >>\n> > \n> >> I am guessing from the name. Am I guessing OK ? A comment would still be nice.\n> >> Why just these bits ? There are more cap/support indicator bits in that port cmd\n> >> bitfield. So why this particular set of bits ? What do they mean all together ?\n> > \n> > Normally the variable/constant name should be self-content (as the\n> > kernel coding style doc states and what the common sense suggests). So\n> > the reader could correctly guess its purpose/content/value. In this\n> > case PORT_CMD_CAP - means PORT CMD capabilities mask. All of the\n> > possible flags have been set in that mask. There are no more\n> > capabilities in the PORT CMD register left undeclared. That's why the\n> > name is selected the way it is and why I haven't added any comment in\n> > here (what the kernel coding style says about the over-commenting the\n> > code).\n> \n\n> Yes, I understood from the name what it is. What I do NOT understand is\n> why all the feature bits are not there. Why this subset only ? A comment\n> about that would be nice so that the reason for it is not lost.\n\nWell, because it's indeed \"PORT_CMD capabilities mask\", and not features,\nnot setups, not settings, not status flags, etc. As I said all the port\nCapabilities have been listed in that mask:\nPORT_CMD_FBSCP\tBIT(22) - FIS-based Switching Capable Port\nPORT_CMD_ESP\tBIT(21) - External SATA Port\nPORT_CMD_CPD\tBIT(20) - Cold Presence Detect\nPORT_CMD_MPSP\tBIT(19) - Mechanical Presence Switch Attached to Port\nPORT_CMD_HPCP\tBIT(18) - Hot Plug Capable Port\nI've or'ed-them-up in a single mask => PORT_CMD_CAP in order to work\nwith them independently from the rest of the PORT_CMD CSR fields.\n\nUnlike the generic controller CAP/CAP2 registers, which consists of the\ndevice capabilities only, PORT_CMD contains various R/W settings (PM, LED\ndriver, etc), RO status flags (CMD-list running, FIS recv running, etc)\nand amongst other the RO/Wo !port-specific capabilities!. The later ones\nindicate the platform-specific device features. Since the register\ncontains flags with the intermixed nature, I need to have a mask to at\nleast get the capabilities and preserve them between the device\nresets. That's why the PORT_CMD_CAP has been introduced in the\nframework of this patch. Its name was chosen with a reference to the\nCAP registers, see:\nHOST_CAP, HOST_CAP2, and finally my PORT_CMD_CAP.\n\n> \n> > \n> >>\n> >> Sure I can go and read the specs to figure it out. But again, a comment would\n> >> avoid readers of the code to have to decrypt all that.\n> > \n> > If you still insist on having an additional comment. I can add\n> > something like \"/* PORT_CMD capabilities mask */\". Are you ok with it?\n> \n\n> That does not help on its own. The macro name says that already. I would\n> like a note about why only these features are selected.\n\nPlease see the explanation above. I don't see what else to say about\nthat mask, because in short what I said above really means \"PORT_CMD\ncapabilities mask\". So should you have some more clever text, which\nwould be more suitable here, please tell me and I'll add it to the\npatch.\n\nRegarding what you said earlier. In order to fully understand the\nAHCI driver a hacker would always need to read the specs. There is\njust no way to do that effectively enough without the controller\nmanual at hands. And the PORT_CMD capabilities isn't the most\ncomplicated part of the device.\n\n-Sergey\n\n> \n> > \n> > -Sergey\n> > \n> >>\n> >>>\n> >>> -Sergey\n> >>>\n> >>>>\n> >>>>> +\n> >>>>> \t/* PORT_FBS bits */\n> >>>>> \tPORT_FBS_DWE_OFFSET\t= 16, /* FBS device with error offset */\n> >>>>> \tPORT_FBS_ADO_OFFSET\t= 12, /* FBS active dev optimization offset */\n> >>>>\n> >>>>\n> >>>> -- \n> >>>> Damien Le Moal\n> >>>> Western Digital Research\n> >>\n> >>\n> >> -- \n> >> Damien Le Moal\n> >> Western Digital Research\n> \n> \n> -- \n> Damien Le Moal\n> Western Digital Research","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["bilbo.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256\n header.s=20210112 header.b=WvEQLsiq;\n\tdkim-atps=neutral","ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2620:137:e000::1:20; helo=out1.vger.email;\n envelope-from=linux-ide-owner@vger.kernel.org; receiver=)"],"Received":["from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20])\n\tby bilbo.ozlabs.org (Postfix) with ESMTP id 4LQ7s916Zsz9sFw\n\tfor ; Sat, 18 Jun 2022 18:11:08 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S230320AbiFRILG (ORCPT );\n Sat, 18 Jun 2022 04:11:06 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:44668 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S230457AbiFRILC (ORCPT\n ); Sat, 18 Jun 2022 04:11:02 -0400","from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n [IPv6:2a00:1450:4864:20::235])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 279D92B193;\n Sat, 18 Jun 2022 01:11:00 -0700 (PDT)","by mail-lj1-x235.google.com with SMTP id b7so6909916ljr.6;\n Sat, 18 Jun 2022 01:11:00 -0700 (PDT)","from mobilestation ([95.79.189.214])\n by smtp.gmail.com with ESMTPSA id\n y23-20020a196417000000b004791b687235sm910895lfb.119.2022.06.18.01.10.56\n (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n Sat, 18 Jun 2022 01:10:57 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=gmail.com; s=20210112;\n h=date:from:to:cc:subject:message-id:references:mime-version\n :content-disposition:in-reply-to;\n bh=Cgt8r4SWxYEShXV42BthEwq1nWQW0QY6gFYNlkR9jIA=;\n b=WvEQLsiqGkVo54RbG2r1VqNFqkfE+4JCxUE7T5vKrUFZ31O9eMrux2q1XItBJZ729Y\n ktZK6rY5aIr+YAYZ6ym1mbK2aXkSGMg0+KcAPgZOcD8dcrpAM5fMAJ/7GASAVN67DMqF\n qvoxB3SX7ttCHllE90Bz2mMBRl+lKutM6HJKOMiSZpaJ4YXBjYWtt8tlFfTXx2sOLACh\n kcbWCDPUqgxwsCh634pf64OurQKBrBvjPbF6kJVnuhtJu7RRct8zHiAKGzGhq4VN9m4T\n 1LrKpAG+MgBVoMLFDKQfkkzb4jtddZl5Pu/PnThj/lNaroF/TKmN5jMisoB9m+dmy5BR\n SBaA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20210112;\n h=x-gm-message-state:date:from:to:cc:subject:message-id:references\n :mime-version:content-disposition:in-reply-to;\n bh=Cgt8r4SWxYEShXV42BthEwq1nWQW0QY6gFYNlkR9jIA=;\n b=5cKjLo9jN5Gp89fUtQAq8Mu0bgTfaxHAYiZBTMQmt6UN5AonUMYFqHG9ZBqusdQuK+\n jIlx6jM0tPPEY0poxBqaqShZ3INpxVJmx524nd3sV2JJp/lG58XDX4zk2V8W+GWqhUq5\n P1UDUgObWBI94K0UMkttl7Szw0+AlEUNKd0eAc9pb/Ewd4IwH/CPRXaUkwBuV4EnOL7t\n xHF7O+1nFmMkTz16HLSaYv/SYqCuenjWiETs5cL2g29yQ1V+LCUsmypTyV+pFKmuiY/y\n A7kzrBInAkR2cuiBQxCkQszxaUaFZu6Ty6G4aijMxiQNHoRPlQ7P1Sm9F9/omRvEcOxD\n hyDw==","X-Gm-Message-State":"AJIora9OqnTQK9WBE301t+6TdbdY3OpfyQZxQhEUG4Jgrrxj2Ng89VKM\n 8/kEkUzeegKNWtmGnfld+RhP5yK1T0O4g2z4","X-Google-Smtp-Source":"\n AGRyM1til4vYja08UaNonR/BsUxCOZoV3Rbhj5CwEAeRzcemMJIDW5thjfdsb91pJKouQKfYt4GIgg==","X-Received":"by 2002:a05:651c:b24:b0:25a:45d7:4bb8 with SMTP id\n b36-20020a05651c0b2400b0025a45d74bb8mr6671159ljr.361.1655539857976;\n Sat, 18 Jun 2022 01:10:57 -0700 (PDT)","Date":"Sat, 18 Jun 2022 11:10:55 +0300","From":"Serge Semin ","To":"Damien Le Moal ","Cc":"Serge Semin ,\n Hans de Goede ,\n Jens Axboe , Hannes Reinecke ,\n Alexey Malahov ,\n Pavel Parkhomenko ,\n Rob Herring , linux-ide@vger.kernel.org,\n linux-kernel@vger.kernel.org, devicetree@vger.kernel.org","Subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","Message-ID":"<20220618081055.grsrjxa5gqiuhy2i@mobilestation>","References":"<20220610081801.11854-1-Sergey.Semin@baikalelectronics.ru>\n <20220610081801.11854-13-Sergey.Semin@baikalelectronics.ru>\n \n <20220615205819.uiqptkqm5qfdvrbj@mobilestation>\n <903e273a-9dc5-f0df-5391-e96e63318323@opensource.wdc.com>\n <20220617203100.jg2o7ponolaenf6r@mobilestation>\n ","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","X-Spam-Status":"No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,\n DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,\n RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE\n autolearn=ham autolearn_force=no version=3.4.6","X-Spam-Checker-Version":"SpamAssassin 3.4.6 (2021-04-09) on\n lindbergh.monkeyblade.net","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ide@vger.kernel.org"}},{"id":2921438,"web_url":"http://patchwork.ozlabs.org/comment/2921438/","msgid":"<20220628120820.3nprogcpe2l3bcm4@mobilestation>","list_archive_url":null,"date":"2022-06-28T12:08:20","subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","submitter":{"id":70038,"url":"http://patchwork.ozlabs.org/api/people/70038/","name":"Serge Semin","email":"fancer.lancer@gmail.com"},"content":"Damien,\nAny notes to the comments below?\n\n-Sergey\n\nOn Sat, Jun 18, 2022 at 11:10:55AM +0300, Serge Semin wrote:\n> On Sat, Jun 18, 2022 at 03:52:28PM +0900, Damien Le Moal wrote:\n> > On 6/18/22 05:31, Serge Semin wrote:\n> > > On Thu, Jun 16, 2022 at 09:28:18AM +0900, Damien Le Moal wrote:\n> > >> On 2022/06/16 5:58, Serge Semin wrote:\n> > >>> On Tue, Jun 14, 2022 at 05:32:41PM +0900, Damien Le Moal wrote:\n> > >>>> On 6/10/22 17:17, Serge Semin wrote:\n> > >>>>> Currently not all of the Port-specific capabilities listed in the\n> > >>>>\n> > >>>> s/listed/are listed\n> > >>>>\n> > >>>>> PORT_CMD-enumeration. Let's extend that set with the Cold Presence\n> > >>>>> Detection and Mechanical Presence Switch attached to the Port flags [1] so\n> > >>>>> to closeup the set of the platform-specific port-capabilities flags. Note\n> > >>>>> these flags are supposed to be set by the platform firmware if there is\n> > >>>>> one. Alternatively as we are about to do they can be set by means of the\n> > >>>>> OF properties.\n> > >>>>>\n> > >>>>> While at it replace PORT_IRQ_DEV_ILCK with PORT_IRQ_DMPS and fix the\n> > >>>>> comment there. In accordance with [2] that IRQ flag is supposed to\n> > >>>>> indicate the state of the signal coming from the Mechanical Presence\n> > >>>>> Switch.\n> > >>>>>\n> > >>>>> [1] Serial ATA AHCI 1.3.1 Specification, p.27\n> > >>>>> [2] Serial ATA AHCI 1.3.1 Specification, p.24, p.88\n> > >>>>>\n> > >>>>> Signed-off-by: Serge Semin \n> > >>>>> Reviewed-by: Hannes Reinecke \n> > >>>>>\n> > >>>>> ---\n> > >>>>>\n> > >>>>> Changelog v4:\n> > >>>>> - Fix the DMPS macros name in the patch log. (@Sergei Shtylyov)\n> > >>>>> ---\n> > >>>>> drivers/ata/ahci.h | 7 ++++++-\n> > >>>>> 1 file changed, 6 insertions(+), 1 deletion(-)\n> > >>>>>\n> > >>>>> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h\n> > >>>>> index 7d834deefeb9..f501531bd1b3 100644\n> > >>>>> --- a/drivers/ata/ahci.h\n> > >>>>> +++ b/drivers/ata/ahci.h\n> > >>>>> @@ -138,7 +138,7 @@ enum {\n> > >>>>> \tPORT_IRQ_BAD_PMP\t= (1 << 23), /* incorrect port multiplier */\n> > >>>>> \n> > >>>>> \tPORT_IRQ_PHYRDY\t\t= (1 << 22), /* PhyRdy changed */\n> > >>>>> -\tPORT_IRQ_DEV_ILCK\t= (1 << 7), /* device interlock */\n> > >>>>> +\tPORT_IRQ_DMPS\t\t= (1 << 7), /* mechanical presence status */\n> > >>>>> \tPORT_IRQ_CONNECT\t= (1 << 6), /* port connect change status */\n> > >>>>> \tPORT_IRQ_SG_DONE\t= (1 << 5), /* descriptor processed */\n> > >>>>> \tPORT_IRQ_UNK_FIS\t= (1 << 4), /* unknown FIS rx'd */\n> > >>>>> @@ -166,6 +166,8 @@ enum {\n> > >>>>> \tPORT_CMD_ATAPI\t\t= (1 << 24), /* Device is ATAPI */\n> > >>>>> \tPORT_CMD_FBSCP\t\t= (1 << 22), /* FBS Capable Port */\n> > >>>>> \tPORT_CMD_ESP\t\t= (1 << 21), /* External Sata Port */\n> > >>>>> +\tPORT_CMD_CPD\t\t= (1 << 20), /* Cold Presence Detection */\n> > >>>>> +\tPORT_CMD_MPSP\t\t= (1 << 19), /* Mechanical Presence Switch */\n> > >>>>> \tPORT_CMD_HPCP\t\t= (1 << 18), /* HotPlug Capable Port */\n> > >>>>> \tPORT_CMD_PMP\t\t= (1 << 17), /* PMP attached */\n> > >>>>> \tPORT_CMD_LIST_ON\t= (1 << 15), /* cmd list DMA engine running */\n> > >>>>> @@ -181,6 +183,9 @@ enum {\n> > >>>>> \tPORT_CMD_ICC_PARTIAL\t= (0x2 << 28), /* Put i/f in partial state */\n> > >>>>> \tPORT_CMD_ICC_SLUMBER\t= (0x6 << 28), /* Put i/f in slumber state */\n> > >>>>> \n> > >>>>> +\tPORT_CMD_CAP\t\t= PORT_CMD_HPCP | PORT_CMD_MPSP |\n> > >>>>> +\t\t\t\t PORT_CMD_CPD | PORT_CMD_ESP | PORT_CMD_FBSCP,\n> > >>>>\n> > >>>\n> > >>>> What is this one for ? A comment above it would be nice.\n> > >>>\n> > >>> Isn't it obviously inferrable from the definition and the item name?\n> > >>\n> > > \n> > >> I am guessing from the name. Am I guessing OK ? A comment would still be nice.\n> > >> Why just these bits ? There are more cap/support indicator bits in that port cmd\n> > >> bitfield. So why this particular set of bits ? What do they mean all together ?\n> > > \n> > > Normally the variable/constant name should be self-content (as the\n> > > kernel coding style doc states and what the common sense suggests). So\n> > > the reader could correctly guess its purpose/content/value. In this\n> > > case PORT_CMD_CAP - means PORT CMD capabilities mask. All of the\n> > > possible flags have been set in that mask. There are no more\n> > > capabilities in the PORT CMD register left undeclared. That's why the\n> > > name is selected the way it is and why I haven't added any comment in\n> > > here (what the kernel coding style says about the over-commenting the\n> > > code).\n> > \n> \n> > Yes, I understood from the name what it is. What I do NOT understand is\n> > why all the feature bits are not there. Why this subset only ? A comment\n> > about that would be nice so that the reason for it is not lost.\n> \n> Well, because it's indeed \"PORT_CMD capabilities mask\", and not features,\n> not setups, not settings, not status flags, etc. As I said all the port\n> Capabilities have been listed in that mask:\n> PORT_CMD_FBSCP\tBIT(22) - FIS-based Switching Capable Port\n> PORT_CMD_ESP\tBIT(21) - External SATA Port\n> PORT_CMD_CPD\tBIT(20) - Cold Presence Detect\n> PORT_CMD_MPSP\tBIT(19) - Mechanical Presence Switch Attached to Port\n> PORT_CMD_HPCP\tBIT(18) - Hot Plug Capable Port\n> I've or'ed-them-up in a single mask => PORT_CMD_CAP in order to work\n> with them independently from the rest of the PORT_CMD CSR fields.\n> \n> Unlike the generic controller CAP/CAP2 registers, which consists of the\n> device capabilities only, PORT_CMD contains various R/W settings (PM, LED\n> driver, etc), RO status flags (CMD-list running, FIS recv running, etc)\n> and amongst other the RO/Wo !port-specific capabilities!. The later ones\n> indicate the platform-specific device features. Since the register\n> contains flags with the intermixed nature, I need to have a mask to at\n> least get the capabilities and preserve them between the device\n> resets. That's why the PORT_CMD_CAP has been introduced in the\n> framework of this patch. Its name was chosen with a reference to the\n> CAP registers, see:\n> HOST_CAP, HOST_CAP2, and finally my PORT_CMD_CAP.\n> \n> > \n> > > \n> > >>\n> > >> Sure I can go and read the specs to figure it out. But again, a comment would\n> > >> avoid readers of the code to have to decrypt all that.\n> > > \n> > > If you still insist on having an additional comment. I can add\n> > > something like \"/* PORT_CMD capabilities mask */\". Are you ok with it?\n> > \n> \n> > That does not help on its own. The macro name says that already. I would\n> > like a note about why only these features are selected.\n> \n> Please see the explanation above. I don't see what else to say about\n> that mask, because in short what I said above really means \"PORT_CMD\n> capabilities mask\". So should you have some more clever text, which\n> would be more suitable here, please tell me and I'll add it to the\n> patch.\n> \n> Regarding what you said earlier. In order to fully understand the\n> AHCI driver a hacker would always need to read the specs. There is\n> just no way to do that effectively enough without the controller\n> manual at hands. And the PORT_CMD capabilities isn't the most\n> complicated part of the device.\n> \n> -Sergey\n> \n> > \n> > > \n> > > -Sergey\n> > > \n> > >>\n> > >>>\n> > >>> -Sergey\n> > >>>\n> > >>>>\n> > >>>>> +\n> > >>>>> \t/* PORT_FBS bits */\n> > >>>>> \tPORT_FBS_DWE_OFFSET\t= 16, /* FBS device with error offset */\n> > >>>>> \tPORT_FBS_ADO_OFFSET\t= 12, /* FBS active dev optimization offset */\n> > >>>>\n> > >>>>\n> > >>>> -- \n> > >>>> Damien Le Moal\n> > >>>> Western Digital Research\n> > >>\n> > >>\n> > >> -- \n> > >> Damien Le Moal\n> > >> Western Digital Research\n> > \n> > \n> > -- \n> > Damien Le Moal\n> > Western Digital Research","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["bilbo.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256\n header.s=20210112 header.b=ZENmrFoC;\n\tdkim-atps=neutral","ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2620:137:e000::1:20; helo=out1.vger.email;\n envelope-from=linux-ide-owner@vger.kernel.org; receiver=)"],"Received":["from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20])\n\tby bilbo.ozlabs.org (Postfix) with ESMTP id 4LXNfR3bkdz9sGC\n\tfor ; Tue, 28 Jun 2022 22:08:31 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S1344255AbiF1MIa (ORCPT );\n Tue, 28 Jun 2022 08:08:30 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:38100 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S238286AbiF1MI2 (ORCPT\n ); Tue, 28 Jun 2022 08:08:28 -0400","from mail-lj1-x229.google.com (mail-lj1-x229.google.com\n [IPv6:2a00:1450:4864:20::229])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4636524F11;\n Tue, 28 Jun 2022 05:08:25 -0700 (PDT)","by mail-lj1-x229.google.com with SMTP id c15so1692105ljr.0;\n Tue, 28 Jun 2022 05:08:25 -0700 (PDT)","from mobilestation ([95.79.140.178])\n by smtp.gmail.com with ESMTPSA id\n i17-20020a2ea231000000b002555d6ca497sm1769938ljm.115.2022.06.28.05.08.22\n (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n Tue, 28 Jun 2022 05:08:22 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=gmail.com; s=20210112;\n h=date:from:to:cc:subject:message-id:references:mime-version\n :content-disposition:in-reply-to;\n bh=wOMPHZxTAXk49D3k8YxdoXS022ibQ+MZGCBPChiaDtU=;\n b=ZENmrFoCpujFuHEP87dUs9OH4WML1Oe2tGWPfEo3b0haXjrbvFCot7uCZr4qq57B3G\n Fw9GjhZ9fB4dwseVV7boti4KaPS3Y8GeLtuWe8ECnVdyOJDjRjuIWqkVXNlaMPQKJmmT\n 4WFtxuNQIj5qvEeWXvIb2mkPk3nuQh+wspP2QQuVpgs49THdp494LnBmrmohQrYumEbF\n Je0rP0o12u69La13Il6SX0+L2kaCfFkl4oG9FJgEkFqSBVsEH8wPBYn2tgfmu+Q7ueoa\n ZGPhwlJl7wbYrWQSDGut82qFkT324ULuWILLlYWaIPShhgcVDQHX+CeXf0A2dtz0dod3\n TT3w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20210112;\n h=x-gm-message-state:date:from:to:cc:subject:message-id:references\n :mime-version:content-disposition:in-reply-to;\n bh=wOMPHZxTAXk49D3k8YxdoXS022ibQ+MZGCBPChiaDtU=;\n b=KlxoiWCLMGBPIic4m5FN4fjs9B1Nv1iT0UVDd9XnZikKZKOL8Neti8hCXTb+BilOy0\n /dlABp7XwNSHjSmV7U5GEnhcI2eJoQrtwgeylO/4vZmTpZ5fX718wN/+AoHNCDu0pGrb\n a6xyByx9zC55TRCJKUAWZJs+jSs4BVvP1A7SzOcc9oRpQLb3c07Rw783S5DrS4upBN3c\n m8Q8l1ypzMEcmsO1o3EiqalQdd1FncnXL5l4xd3XSSJud/R7WBEZY3C3mxr1+5BFsma+\n mQz2qBTVeohMQu+sFmb5jXj0KQDlb2mZjBtvnAZpmpHxZ9UUaf4h2rT0U1g7SI0CmDd+\n zGQg==","X-Gm-Message-State":"AJIora9z26osHqolET88KyGhVBxKSctWwgVic1h+/VcsJyWp4iw36+Ao\n +rHaddu+YOK1OAhRWdSEBMWBMMSNZ1jrvQ==","X-Google-Smtp-Source":"\n AGRyM1vWGiUUC8PJAqaqyVLzlhx5Xr5O6ERuzpLNXemJwAmj8Ym3ze3lgJGTd40bOF/3PYR5t0ob8A==","X-Received":"by 2002:a2e:a484:0:b0:25a:8c94:3763 with SMTP id\n h4-20020a2ea484000000b0025a8c943763mr9188084lji.64.1656418103542;\n Tue, 28 Jun 2022 05:08:23 -0700 (PDT)","Date":"Tue, 28 Jun 2022 15:08:20 +0300","From":"Serge Semin ","To":"Damien Le Moal ","Cc":"Serge Semin ,\n Hans de Goede ,\n Jens Axboe , Hannes Reinecke ,\n Alexey Malahov ,\n Pavel Parkhomenko ,\n Rob Herring , linux-ide@vger.kernel.org,\n linux-kernel@vger.kernel.org, devicetree@vger.kernel.org","Subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","Message-ID":"<20220628120820.3nprogcpe2l3bcm4@mobilestation>","References":"<20220610081801.11854-1-Sergey.Semin@baikalelectronics.ru>\n <20220610081801.11854-13-Sergey.Semin@baikalelectronics.ru>\n \n <20220615205819.uiqptkqm5qfdvrbj@mobilestation>\n <903e273a-9dc5-f0df-5391-e96e63318323@opensource.wdc.com>\n <20220617203100.jg2o7ponolaenf6r@mobilestation>\n \n <20220618081055.grsrjxa5gqiuhy2i@mobilestation>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220618081055.grsrjxa5gqiuhy2i@mobilestation>","X-Spam-Status":"No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,\n DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,\n RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE\n autolearn=ham autolearn_force=no version=3.4.6","X-Spam-Checker-Version":"SpamAssassin 3.4.6 (2021-04-09) on\n lindbergh.monkeyblade.net","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ide@vger.kernel.org"}},{"id":2922090,"web_url":"http://patchwork.ozlabs.org/comment/2922090/","msgid":"<3dade607-71a5-28ad-fb0b-f5b75c1b5392@opensource.wdc.com>","list_archive_url":null,"date":"2022-06-29T01:35:13","subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","submitter":{"id":82259,"url":"http://patchwork.ozlabs.org/api/people/82259/","name":"Damien Le Moal","email":"damien.lemoal@opensource.wdc.com"},"content":"On 6/28/22 21:08, Serge Semin wrote:\n> Damien,\n> Any notes to the comments below?\n\nBeen very busy and had no time to look at this. Please post your latest\nversion of the series and we'll go from there.\n\n> \n> -Sergey\n> \n> On Sat, Jun 18, 2022 at 11:10:55AM +0300, Serge Semin wrote:\n>> On Sat, Jun 18, 2022 at 03:52:28PM +0900, Damien Le Moal wrote:\n>>> On 6/18/22 05:31, Serge Semin wrote:\n>>>> On Thu, Jun 16, 2022 at 09:28:18AM +0900, Damien Le Moal wrote:\n>>>>> On 2022/06/16 5:58, Serge Semin wrote:\n>>>>>> On Tue, Jun 14, 2022 at 05:32:41PM +0900, Damien Le Moal wrote:\n>>>>>>> On 6/10/22 17:17, Serge Semin wrote:\n>>>>>>>> Currently not all of the Port-specific capabilities listed in the\n>>>>>>>\n>>>>>>> s/listed/are listed\n>>>>>>>\n>>>>>>>> PORT_CMD-enumeration. Let's extend that set with the Cold Presence\n>>>>>>>> Detection and Mechanical Presence Switch attached to the Port flags [1] so\n>>>>>>>> to closeup the set of the platform-specific port-capabilities flags. Note\n>>>>>>>> these flags are supposed to be set by the platform firmware if there is\n>>>>>>>> one. Alternatively as we are about to do they can be set by means of the\n>>>>>>>> OF properties.\n>>>>>>>>\n>>>>>>>> While at it replace PORT_IRQ_DEV_ILCK with PORT_IRQ_DMPS and fix the\n>>>>>>>> comment there. In accordance with [2] that IRQ flag is supposed to\n>>>>>>>> indicate the state of the signal coming from the Mechanical Presence\n>>>>>>>> Switch.\n>>>>>>>>\n>>>>>>>> [1] Serial ATA AHCI 1.3.1 Specification, p.27\n>>>>>>>> [2] Serial ATA AHCI 1.3.1 Specification, p.24, p.88\n>>>>>>>>\n>>>>>>>> Signed-off-by: Serge Semin \n>>>>>>>> Reviewed-by: Hannes Reinecke \n>>>>>>>>\n>>>>>>>> ---\n>>>>>>>>\n>>>>>>>> Changelog v4:\n>>>>>>>> - Fix the DMPS macros name in the patch log. (@Sergei Shtylyov)\n>>>>>>>> ---\n>>>>>>>> drivers/ata/ahci.h | 7 ++++++-\n>>>>>>>> 1 file changed, 6 insertions(+), 1 deletion(-)\n>>>>>>>>\n>>>>>>>> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h\n>>>>>>>> index 7d834deefeb9..f501531bd1b3 100644\n>>>>>>>> --- a/drivers/ata/ahci.h\n>>>>>>>> +++ b/drivers/ata/ahci.h\n>>>>>>>> @@ -138,7 +138,7 @@ enum {\n>>>>>>>> \tPORT_IRQ_BAD_PMP\t= (1 << 23), /* incorrect port multiplier */\n>>>>>>>> \n>>>>>>>> \tPORT_IRQ_PHYRDY\t\t= (1 << 22), /* PhyRdy changed */\n>>>>>>>> -\tPORT_IRQ_DEV_ILCK\t= (1 << 7), /* device interlock */\n>>>>>>>> +\tPORT_IRQ_DMPS\t\t= (1 << 7), /* mechanical presence status */\n>>>>>>>> \tPORT_IRQ_CONNECT\t= (1 << 6), /* port connect change status */\n>>>>>>>> \tPORT_IRQ_SG_DONE\t= (1 << 5), /* descriptor processed */\n>>>>>>>> \tPORT_IRQ_UNK_FIS\t= (1 << 4), /* unknown FIS rx'd */\n>>>>>>>> @@ -166,6 +166,8 @@ enum {\n>>>>>>>> \tPORT_CMD_ATAPI\t\t= (1 << 24), /* Device is ATAPI */\n>>>>>>>> \tPORT_CMD_FBSCP\t\t= (1 << 22), /* FBS Capable Port */\n>>>>>>>> \tPORT_CMD_ESP\t\t= (1 << 21), /* External Sata Port */\n>>>>>>>> +\tPORT_CMD_CPD\t\t= (1 << 20), /* Cold Presence Detection */\n>>>>>>>> +\tPORT_CMD_MPSP\t\t= (1 << 19), /* Mechanical Presence Switch */\n>>>>>>>> \tPORT_CMD_HPCP\t\t= (1 << 18), /* HotPlug Capable Port */\n>>>>>>>> \tPORT_CMD_PMP\t\t= (1 << 17), /* PMP attached */\n>>>>>>>> \tPORT_CMD_LIST_ON\t= (1 << 15), /* cmd list DMA engine running */\n>>>>>>>> @@ -181,6 +183,9 @@ enum {\n>>>>>>>> \tPORT_CMD_ICC_PARTIAL\t= (0x2 << 28), /* Put i/f in partial state */\n>>>>>>>> \tPORT_CMD_ICC_SLUMBER\t= (0x6 << 28), /* Put i/f in slumber state */\n>>>>>>>> \n>>>>>>>> +\tPORT_CMD_CAP\t\t= PORT_CMD_HPCP | PORT_CMD_MPSP |\n>>>>>>>> +\t\t\t\t PORT_CMD_CPD | PORT_CMD_ESP | PORT_CMD_FBSCP,\n>>>>>>>\n>>>>>>\n>>>>>>> What is this one for ? A comment above it would be nice.\n>>>>>>\n>>>>>> Isn't it obviously inferrable from the definition and the item name?\n>>>>>\n>>>>\n>>>>> I am guessing from the name. Am I guessing OK ? A comment would still be nice.\n>>>>> Why just these bits ? There are more cap/support indicator bits in that port cmd\n>>>>> bitfield. So why this particular set of bits ? What do they mean all together ?\n>>>>\n>>>> Normally the variable/constant name should be self-content (as the\n>>>> kernel coding style doc states and what the common sense suggests). So\n>>>> the reader could correctly guess its purpose/content/value. In this\n>>>> case PORT_CMD_CAP - means PORT CMD capabilities mask. All of the\n>>>> possible flags have been set in that mask. There are no more\n>>>> capabilities in the PORT CMD register left undeclared. That's why the\n>>>> name is selected the way it is and why I haven't added any comment in\n>>>> here (what the kernel coding style says about the over-commenting the\n>>>> code).\n>>>\n>>\n>>> Yes, I understood from the name what it is. What I do NOT understand is\n>>> why all the feature bits are not there. Why this subset only ? A comment\n>>> about that would be nice so that the reason for it is not lost.\n>>\n>> Well, because it's indeed \"PORT_CMD capabilities mask\", and not features,\n>> not setups, not settings, not status flags, etc. As I said all the port\n>> Capabilities have been listed in that mask:\n>> PORT_CMD_FBSCP\tBIT(22) - FIS-based Switching Capable Port\n>> PORT_CMD_ESP\tBIT(21) - External SATA Port\n>> PORT_CMD_CPD\tBIT(20) - Cold Presence Detect\n>> PORT_CMD_MPSP\tBIT(19) - Mechanical Presence Switch Attached to Port\n>> PORT_CMD_HPCP\tBIT(18) - Hot Plug Capable Port\n>> I've or'ed-them-up in a single mask => PORT_CMD_CAP in order to work\n>> with them independently from the rest of the PORT_CMD CSR fields.\n>>\n>> Unlike the generic controller CAP/CAP2 registers, which consists of the\n>> device capabilities only, PORT_CMD contains various R/W settings (PM, LED\n>> driver, etc), RO status flags (CMD-list running, FIS recv running, etc)\n>> and amongst other the RO/Wo !port-specific capabilities!. The later ones\n>> indicate the platform-specific device features. Since the register\n>> contains flags with the intermixed nature, I need to have a mask to at\n>> least get the capabilities and preserve them between the device\n>> resets. That's why the PORT_CMD_CAP has been introduced in the\n>> framework of this patch. Its name was chosen with a reference to the\n>> CAP registers, see:\n>> HOST_CAP, HOST_CAP2, and finally my PORT_CMD_CAP.\n>>\n>>>\n>>>>\n>>>>>\n>>>>> Sure I can go and read the specs to figure it out. But again, a comment would\n>>>>> avoid readers of the code to have to decrypt all that.\n>>>>\n>>>> If you still insist on having an additional comment. I can add\n>>>> something like \"/* PORT_CMD capabilities mask */\". Are you ok with it?\n>>>\n>>\n>>> That does not help on its own. The macro name says that already. I would\n>>> like a note about why only these features are selected.\n>>\n>> Please see the explanation above. I don't see what else to say about\n>> that mask, because in short what I said above really means \"PORT_CMD\n>> capabilities mask\". So should you have some more clever text, which\n>> would be more suitable here, please tell me and I'll add it to the\n>> patch.\n>>\n>> Regarding what you said earlier. In order to fully understand the\n>> AHCI driver a hacker would always need to read the specs. There is\n>> just no way to do that effectively enough without the controller\n>> manual at hands. And the PORT_CMD capabilities isn't the most\n>> complicated part of the device.\n>>\n>> -Sergey\n>>\n>>>\n>>>>\n>>>> -Sergey\n>>>>\n>>>>>\n>>>>>>\n>>>>>> -Sergey\n>>>>>>\n>>>>>>>\n>>>>>>>> +\n>>>>>>>> \t/* PORT_FBS bits */\n>>>>>>>> \tPORT_FBS_DWE_OFFSET\t= 16, /* FBS device with error offset */\n>>>>>>>> \tPORT_FBS_ADO_OFFSET\t= 12, /* FBS active dev optimization offset */\n>>>>>>>\n>>>>>>>\n>>>>>>> -- \n>>>>>>> Damien Le Moal\n>>>>>>> Western Digital Research\n>>>>>\n>>>>>\n>>>>> -- \n>>>>> Damien Le Moal\n>>>>> Western Digital Research\n>>>\n>>>\n>>> -- \n>>> Damien Le Moal\n>>> Western Digital Research","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["bilbo.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n unprotected) header.d=wdc.com header.i=@wdc.com header.a=rsa-sha256\n header.s=dkim.wdc.com header.b=mv6TttMa;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=opensource.wdc.com header.i=@opensource.wdc.com\n header.a=rsa-sha256 header.s=dkim header.b=OmeTcmBg;\n\tdkim-atps=neutral","ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2620:137:e000::1:20; helo=out1.vger.email;\n envelope-from=linux-ide-owner@vger.kernel.org; receiver=)","usg-ed-osssrv.wdc.com (amavisd-new); dkim=pass\n reason=\"pass (just generated, assumed good)\"\n header.d=opensource.wdc.com"],"Received":["from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20])\n\tby bilbo.ozlabs.org (Postfix) with ESMTP id 4LXkYS6JY3z9sGZ\n\tfor ; Wed, 29 Jun 2022 11:35:24 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S229681AbiF2BfV (ORCPT );\n Tue, 28 Jun 2022 21:35:21 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:35762 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S229547AbiF2BfV (ORCPT\n ); Tue, 28 Jun 2022 21:35:21 -0400","from esa4.hgst.iphmx.com (esa4.hgst.iphmx.com [216.71.154.42])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 97BD625C7C\n for ; Tue, 28 Jun 2022 18:35:19 -0700 (PDT)","from uls-op-cesaip02.wdc.com (HELO uls-op-cesaep02.wdc.com)\n ([199.255.45.15])\n by ob1.hgst.iphmx.com with ESMTP; 29 Jun 2022 09:35:18 +0800","from uls-op-cesaip02.wdc.com ([10.248.3.37])\n by uls-op-cesaep02.wdc.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256;\n 28 Jun 2022 17:52:57 -0700","from usg-ed-osssrv.wdc.com ([10.3.10.180])\n by uls-op-cesaip02.wdc.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256;\n 28 Jun 2022 18:35:19 -0700","from usg-ed-osssrv.wdc.com (usg-ed-osssrv.wdc.com [127.0.0.1])\n by usg-ed-osssrv.wdc.com (Postfix) with ESMTP id 4LXkYL1xdvz1Rwnx\n for ; Tue, 28 Jun 2022 18:35:18 -0700 (PDT)","from usg-ed-osssrv.wdc.com ([127.0.0.1])\n by usg-ed-osssrv.wdc.com (usg-ed-osssrv.wdc.com [127.0.0.1])\n (amavisd-new, port 10026)\n with ESMTP id qq_w-vsSpkLX for ;\n Tue, 28 Jun 2022 18:35:16 -0700 (PDT)","from [10.225.163.99] (unknown [10.225.163.99])\n by usg-ed-osssrv.wdc.com (Postfix) with ESMTPSA id 4LXkYG5x5vz1RtVk;\n Tue, 28 Jun 2022 18:35:14 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=simple/simple;\n d=wdc.com; i=@wdc.com; q=dns/txt; s=dkim.wdc.com;\n t=1656466519; x=1688002519;\n h=message-id:date:mime-version:subject:to:cc:references:\n from:in-reply-to:content-transfer-encoding;\n bh=s/ZcR4rZ2s2VxX2a+v+kNduBcbXrq+qY3otwjZQxkI8=;\n b=mv6TttMaeyGtL5pInW/ApBBQosEtRHO4tPAyDOyMKjTw4cvpjsEOBRW9\n HD9JEfbtzTJcgd0qqYcYDI547Rs+VvObJtQiuMfVfRuMCAgVrERSUe+NG\n ZS+GL4gHnhoYm64JeQgM0gljARoox+McJIDuTC9NrhxQ1evyjh77ng2PX\n xejxtef7TXRw2oNInEZY1XRFXjmurEUEGPaMOwW9tZIhleVJJIUVhvAGX\n LhGcJZZMazR4eGzfkjr9GV+O/BK5ynJTiFU1BBJArJiIlo9qOKfB6n6Rp\n I2yRz+Wqq4r1fw7slcPKKReanNxUGpotUXhTuaUmOiS3GlOXLzM9EXXl9\n A==;","v=1; a=rsa-sha256; c=relaxed/simple; d=\n opensource.wdc.com; h=content-transfer-encoding:content-type\n :in-reply-to:organization:from:references:to:content-language\n :subject:user-agent:mime-version:date:message-id; s=dkim; t=\n 1656466516; x=1659058517; bh=s/ZcR4rZ2s2VxX2a+v+kNduBcbXrq+qY3ot\n wjZQxkI8=; b=OmeTcmBgcAZrnVoZTECVFgEfp9RwKPbEgdJUcSqX4PVm0TaRTaz\n 1tErG/4HgZKYjBqGsGZ19zTSkypuS5zKKrqYOSBRtWhkxftZoe6shd6MWeNJFqvK\n WPV6lZuYkZdo5+R/+wCJ58+C9FTWmcu6GVfPIDl5umJEXCg65xkI3XhcvxiZUZXK\n NUQ4SKY4wjw2vxGOKgF1jZcEGukCoqWeAcZgOu95Eu9+NnLopNANsn5AFPK/ce0J\n hJ1EAb15HS/rzSx6SpUbrdd0gZunylqO2pmmlOnPFrAfg2DlDEHm6yRjk18vFc6s\n 1nQKrlae7I4bFc21qpizbg//DRPfFPKQ3bA=="],"X-IronPort-AV":"E=Sophos;i=\"5.92,230,1650902400\";\n d=\"scan'208\";a=\"203017038\"","IronPort-SDR":["\n 4c780DRr1H246Oi+IqTftm2p7hWDlnFSBdxRI+LVDMvp2zUY21i53ZQfO3xoshZ4/5vRbxRIwW\n XMgZFQi0XbWgLM3njHJKHGGZeVSRpM+llr6rOIrnOpY+dIixCiZkDJk4tiIP9XwijJHYQM6Y4F\n 2c+GFTE57ecG12sQoagKvfRz6uiSyIQGisaD0qAWLHTztCcqv1c5NBeJh1cKP3TA1AzllYOBww\n aJwkJOCj4CUbsawVdLz0xPo9GoxquRfie0Pv0lHCoWqHBYW4z/fi+/scfGTF0ZT9zBYIbea4wg\n 5Fff4T2T4QDWwK/XkRl0HaE5","\n H+dngeX1vNkYPGnw77Z7u59FN5aKNbATN0gl/I21Q4qpZeWyVgjSK/LKq76K8lFl1oEMwmE7jl\n MoJJ89eNrn3goYbRDsi3ZaELMMd3F/+7jxeEjshmJHJWuVfQze+ADNJO67/gDMpgmlkb3vflgi\n mIX8OoOPlvZWI4BumSLRndLh9CVUaaioiHcDRvNKyBeB5WVrPO1nJQK0AIYQcaTd8z0JUjlN9M\n oxcL9S3HREGCJJ6qni5DRAh2HIHsNNjiOfpQiVej4Dv7RDqOjYzDq5sEVS2BiObS26mZH6AsKY\n syw="],"WDCIronportException":"Internal","X-Virus-Scanned":"amavisd-new at usg-ed-osssrv.wdc.com","Message-ID":"<3dade607-71a5-28ad-fb0b-f5b75c1b5392@opensource.wdc.com>","Date":"Wed, 29 Jun 2022 10:35:13 +0900","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n Thunderbird/91.10.0","Subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","Content-Language":"en-US","To":"Serge Semin ","Cc":"Serge Semin ,\n Hans de Goede ,\n Jens Axboe , Hannes Reinecke ,\n Alexey Malahov ,\n Pavel Parkhomenko ,\n Rob Herring , linux-ide@vger.kernel.org,\n linux-kernel@vger.kernel.org, devicetree@vger.kernel.org","References":"<20220610081801.11854-1-Sergey.Semin@baikalelectronics.ru>\n <20220610081801.11854-13-Sergey.Semin@baikalelectronics.ru>\n \n <20220615205819.uiqptkqm5qfdvrbj@mobilestation>\n <903e273a-9dc5-f0df-5391-e96e63318323@opensource.wdc.com>\n <20220617203100.jg2o7ponolaenf6r@mobilestation>\n \n <20220618081055.grsrjxa5gqiuhy2i@mobilestation>\n <20220628120820.3nprogcpe2l3bcm4@mobilestation>","From":"Damien Le Moal ","Organization":"Western Digital Research","In-Reply-To":"<20220628120820.3nprogcpe2l3bcm4@mobilestation>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","X-Spam-Status":"No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,\n DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_MED,\n SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham\n autolearn_force=no version=3.4.6","X-Spam-Checker-Version":"SpamAssassin 3.4.6 (2021-04-09) on\n lindbergh.monkeyblade.net","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ide@vger.kernel.org"}},{"id":2922098,"web_url":"http://patchwork.ozlabs.org/comment/2922098/","msgid":"<20220629014742.bajkuosd752pg2po@mobilestation>","list_archive_url":null,"date":"2022-06-29T01:47:42","subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","submitter":{"id":70038,"url":"http://patchwork.ozlabs.org/api/people/70038/","name":"Serge Semin","email":"fancer.lancer@gmail.com"},"content":"On Wed, Jun 29, 2022 at 10:35:13AM +0900, Damien Le Moal wrote:\n> On 6/28/22 21:08, Serge Semin wrote:\n> > Damien,\n> > Any notes to the comments below?\n> \n\n> Been very busy and had no time to look at this. Please post your latest\n> version of the series and we'll go from there.\n\nOk. As soon as I get the responses from Rob.\n\n-Sergey\n\n> \n> > \n> > -Sergey\n> > \n> > On Sat, Jun 18, 2022 at 11:10:55AM +0300, Serge Semin wrote:\n> >> On Sat, Jun 18, 2022 at 03:52:28PM +0900, Damien Le Moal wrote:\n> >>> On 6/18/22 05:31, Serge Semin wrote:\n> >>>> On Thu, Jun 16, 2022 at 09:28:18AM +0900, Damien Le Moal wrote:\n> >>>>> On 2022/06/16 5:58, Serge Semin wrote:\n> >>>>>> On Tue, Jun 14, 2022 at 05:32:41PM +0900, Damien Le Moal wrote:\n> >>>>>>> On 6/10/22 17:17, Serge Semin wrote:\n> >>>>>>>> Currently not all of the Port-specific capabilities listed in the\n> >>>>>>>\n> >>>>>>> s/listed/are listed\n> >>>>>>>\n> >>>>>>>> PORT_CMD-enumeration. Let's extend that set with the Cold Presence\n> >>>>>>>> Detection and Mechanical Presence Switch attached to the Port flags [1] so\n> >>>>>>>> to closeup the set of the platform-specific port-capabilities flags. Note\n> >>>>>>>> these flags are supposed to be set by the platform firmware if there is\n> >>>>>>>> one. Alternatively as we are about to do they can be set by means of the\n> >>>>>>>> OF properties.\n> >>>>>>>>\n> >>>>>>>> While at it replace PORT_IRQ_DEV_ILCK with PORT_IRQ_DMPS and fix the\n> >>>>>>>> comment there. In accordance with [2] that IRQ flag is supposed to\n> >>>>>>>> indicate the state of the signal coming from the Mechanical Presence\n> >>>>>>>> Switch.\n> >>>>>>>>\n> >>>>>>>> [1] Serial ATA AHCI 1.3.1 Specification, p.27\n> >>>>>>>> [2] Serial ATA AHCI 1.3.1 Specification, p.24, p.88\n> >>>>>>>>\n> >>>>>>>> Signed-off-by: Serge Semin \n> >>>>>>>> Reviewed-by: Hannes Reinecke \n> >>>>>>>>\n> >>>>>>>> ---\n> >>>>>>>>\n> >>>>>>>> Changelog v4:\n> >>>>>>>> - Fix the DMPS macros name in the patch log. (@Sergei Shtylyov)\n> >>>>>>>> ---\n> >>>>>>>> drivers/ata/ahci.h | 7 ++++++-\n> >>>>>>>> 1 file changed, 6 insertions(+), 1 deletion(-)\n> >>>>>>>>\n> >>>>>>>> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h\n> >>>>>>>> index 7d834deefeb9..f501531bd1b3 100644\n> >>>>>>>> --- a/drivers/ata/ahci.h\n> >>>>>>>> +++ b/drivers/ata/ahci.h\n> >>>>>>>> @@ -138,7 +138,7 @@ enum {\n> >>>>>>>> \tPORT_IRQ_BAD_PMP\t= (1 << 23), /* incorrect port multiplier */\n> >>>>>>>> \n> >>>>>>>> \tPORT_IRQ_PHYRDY\t\t= (1 << 22), /* PhyRdy changed */\n> >>>>>>>> -\tPORT_IRQ_DEV_ILCK\t= (1 << 7), /* device interlock */\n> >>>>>>>> +\tPORT_IRQ_DMPS\t\t= (1 << 7), /* mechanical presence status */\n> >>>>>>>> \tPORT_IRQ_CONNECT\t= (1 << 6), /* port connect change status */\n> >>>>>>>> \tPORT_IRQ_SG_DONE\t= (1 << 5), /* descriptor processed */\n> >>>>>>>> \tPORT_IRQ_UNK_FIS\t= (1 << 4), /* unknown FIS rx'd */\n> >>>>>>>> @@ -166,6 +166,8 @@ enum {\n> >>>>>>>> \tPORT_CMD_ATAPI\t\t= (1 << 24), /* Device is ATAPI */\n> >>>>>>>> \tPORT_CMD_FBSCP\t\t= (1 << 22), /* FBS Capable Port */\n> >>>>>>>> \tPORT_CMD_ESP\t\t= (1 << 21), /* External Sata Port */\n> >>>>>>>> +\tPORT_CMD_CPD\t\t= (1 << 20), /* Cold Presence Detection */\n> >>>>>>>> +\tPORT_CMD_MPSP\t\t= (1 << 19), /* Mechanical Presence Switch */\n> >>>>>>>> \tPORT_CMD_HPCP\t\t= (1 << 18), /* HotPlug Capable Port */\n> >>>>>>>> \tPORT_CMD_PMP\t\t= (1 << 17), /* PMP attached */\n> >>>>>>>> \tPORT_CMD_LIST_ON\t= (1 << 15), /* cmd list DMA engine running */\n> >>>>>>>> @@ -181,6 +183,9 @@ enum {\n> >>>>>>>> \tPORT_CMD_ICC_PARTIAL\t= (0x2 << 28), /* Put i/f in partial state */\n> >>>>>>>> \tPORT_CMD_ICC_SLUMBER\t= (0x6 << 28), /* Put i/f in slumber state */\n> >>>>>>>> \n> >>>>>>>> +\tPORT_CMD_CAP\t\t= PORT_CMD_HPCP | PORT_CMD_MPSP |\n> >>>>>>>> +\t\t\t\t PORT_CMD_CPD | PORT_CMD_ESP | PORT_CMD_FBSCP,\n> >>>>>>>\n> >>>>>>\n> >>>>>>> What is this one for ? A comment above it would be nice.\n> >>>>>>\n> >>>>>> Isn't it obviously inferrable from the definition and the item name?\n> >>>>>\n> >>>>\n> >>>>> I am guessing from the name. Am I guessing OK ? A comment would still be nice.\n> >>>>> Why just these bits ? There are more cap/support indicator bits in that port cmd\n> >>>>> bitfield. So why this particular set of bits ? What do they mean all together ?\n> >>>>\n> >>>> Normally the variable/constant name should be self-content (as the\n> >>>> kernel coding style doc states and what the common sense suggests). So\n> >>>> the reader could correctly guess its purpose/content/value. In this\n> >>>> case PORT_CMD_CAP - means PORT CMD capabilities mask. All of the\n> >>>> possible flags have been set in that mask. There are no more\n> >>>> capabilities in the PORT CMD register left undeclared. That's why the\n> >>>> name is selected the way it is and why I haven't added any comment in\n> >>>> here (what the kernel coding style says about the over-commenting the\n> >>>> code).\n> >>>\n> >>\n> >>> Yes, I understood from the name what it is. What I do NOT understand is\n> >>> why all the feature bits are not there. Why this subset only ? A comment\n> >>> about that would be nice so that the reason for it is not lost.\n> >>\n> >> Well, because it's indeed \"PORT_CMD capabilities mask\", and not features,\n> >> not setups, not settings, not status flags, etc. As I said all the port\n> >> Capabilities have been listed in that mask:\n> >> PORT_CMD_FBSCP\tBIT(22) - FIS-based Switching Capable Port\n> >> PORT_CMD_ESP\tBIT(21) - External SATA Port\n> >> PORT_CMD_CPD\tBIT(20) - Cold Presence Detect\n> >> PORT_CMD_MPSP\tBIT(19) - Mechanical Presence Switch Attached to Port\n> >> PORT_CMD_HPCP\tBIT(18) - Hot Plug Capable Port\n> >> I've or'ed-them-up in a single mask => PORT_CMD_CAP in order to work\n> >> with them independently from the rest of the PORT_CMD CSR fields.\n> >>\n> >> Unlike the generic controller CAP/CAP2 registers, which consists of the\n> >> device capabilities only, PORT_CMD contains various R/W settings (PM, LED\n> >> driver, etc), RO status flags (CMD-list running, FIS recv running, etc)\n> >> and amongst other the RO/Wo !port-specific capabilities!. The later ones\n> >> indicate the platform-specific device features. Since the register\n> >> contains flags with the intermixed nature, I need to have a mask to at\n> >> least get the capabilities and preserve them between the device\n> >> resets. That's why the PORT_CMD_CAP has been introduced in the\n> >> framework of this patch. Its name was chosen with a reference to the\n> >> CAP registers, see:\n> >> HOST_CAP, HOST_CAP2, and finally my PORT_CMD_CAP.\n> >>\n> >>>\n> >>>>\n> >>>>>\n> >>>>> Sure I can go and read the specs to figure it out. But again, a comment would\n> >>>>> avoid readers of the code to have to decrypt all that.\n> >>>>\n> >>>> If you still insist on having an additional comment. I can add\n> >>>> something like \"/* PORT_CMD capabilities mask */\". Are you ok with it?\n> >>>\n> >>\n> >>> That does not help on its own. The macro name says that already. I would\n> >>> like a note about why only these features are selected.\n> >>\n> >> Please see the explanation above. I don't see what else to say about\n> >> that mask, because in short what I said above really means \"PORT_CMD\n> >> capabilities mask\". So should you have some more clever text, which\n> >> would be more suitable here, please tell me and I'll add it to the\n> >> patch.\n> >>\n> >> Regarding what you said earlier. In order to fully understand the\n> >> AHCI driver a hacker would always need to read the specs. There is\n> >> just no way to do that effectively enough without the controller\n> >> manual at hands. And the PORT_CMD capabilities isn't the most\n> >> complicated part of the device.\n> >>\n> >> -Sergey\n> >>\n> >>>\n> >>>>\n> >>>> -Sergey\n> >>>>\n> >>>>>\n> >>>>>>\n> >>>>>> -Sergey\n> >>>>>>\n> >>>>>>>\n> >>>>>>>> +\n> >>>>>>>> \t/* PORT_FBS bits */\n> >>>>>>>> \tPORT_FBS_DWE_OFFSET\t= 16, /* FBS device with error offset */\n> >>>>>>>> \tPORT_FBS_ADO_OFFSET\t= 12, /* FBS active dev optimization offset */\n> >>>>>>>\n> >>>>>>>\n> >>>>>>> -- \n> >>>>>>> Damien Le Moal\n> >>>>>>> Western Digital Research\n> >>>>>\n> >>>>>\n> >>>>> -- \n> >>>>> Damien Le Moal\n> >>>>> Western Digital Research\n> >>>\n> >>>\n> >>> -- \n> >>> Damien Le Moal\n> >>> Western Digital Research\n> \n> \n> -- \n> Damien Le Moal\n> Western Digital Research","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["bilbo.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256\n header.s=20210112 header.b=Dmwyr4wS;\n\tdkim-atps=neutral","ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2620:137:e000::1:20; helo=out1.vger.email;\n envelope-from=linux-ide-owner@vger.kernel.org; receiver=)"],"Received":["from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20])\n\tby bilbo.ozlabs.org (Postfix) with ESMTP id 4LXkqp5q7pz9sGZ\n\tfor ; Wed, 29 Jun 2022 11:47:50 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S229780AbiF2Brt (ORCPT );\n Tue, 28 Jun 2022 21:47:49 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:44586 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S229475AbiF2Brt (ORCPT\n ); Tue, 28 Jun 2022 21:47:49 -0400","from mail-lj1-x232.google.com (mail-lj1-x232.google.com\n [IPv6:2a00:1450:4864:20::232])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 559F92C651;\n Tue, 28 Jun 2022 18:47:47 -0700 (PDT)","by mail-lj1-x232.google.com with SMTP id u14so3673189ljh.2;\n Tue, 28 Jun 2022 18:47:47 -0700 (PDT)","from mobilestation ([95.79.140.178])\n by smtp.gmail.com with ESMTPSA id\n u11-20020a05651206cb00b0047f79a0f724sm2411223lff.12.2022.06.28.18.47.44\n (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n Tue, 28 Jun 2022 18:47:44 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=gmail.com; s=20210112;\n h=date:from:to:cc:subject:message-id:references:mime-version\n :content-disposition:in-reply-to;\n bh=Nm/ngXM3qWrrdMSmJV16nVGUE21ajwm62VfuPgXU3gQ=;\n b=Dmwyr4wSUhMlyeAftAposKBusLNX9o5gkdIsFqcE0tefBPhP4zgjfCorX+vIREX5tL\n sL05muH4goEW4RJLGY8OzwW69+RDbuPoDCsdV/HAaxKLzlm5FR2PUk8ZbVJ7LvcJ9qd1\n 1KktREwmDb9seQIuEqLvXTrDBIMXYRnVS6yTZjITzXHrbo7FNCLt4UizA1/bkfkSMbW3\n sLKUxk1GOcm2zGu4vI4PEsLGWX0UNG0bOd8aU5d0sdkgVltOKs4z5P4ZM2wuWERCurCv\n uKj+/88XrpySUGNCPE589fZC28gwfP3LXQq/AM7d16G4dm9Kg8TUxYesAva9i5EVXxfg\n GptQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20210112;\n h=x-gm-message-state:date:from:to:cc:subject:message-id:references\n :mime-version:content-disposition:in-reply-to;\n bh=Nm/ngXM3qWrrdMSmJV16nVGUE21ajwm62VfuPgXU3gQ=;\n b=etx15eHBpSK2kNbbHkY2yBQOVZlUVjYQomjwaEjjg7eVNLYJBkwVl/lIqajjdFvJvS\n tbz3nckbJEDRSruAwLtCrYt6jBbuJh6ufKLRUKa9xHxlW0nKq5rcApQVZb2Jrddrtc2s\n sBUivr/5rIfLoAv03Dts5xVTQ5Uo3EcUzYSXJzJ+49oeqY090VIXw9VXhbw2qc3VIa6t\n I7ozujE1OKMRe/7d/5sffB+QLmXvGpLgPmA3ZmfaNzHPPODImWYThbjaieGrWYQZnEtK\n 7COt8yXPiaSqLUMIhGFgEvfKZzyOtiogMgVbSUiBVa5t/N2Pd/AuBXZ/bK7gXIT4jZn6\n z57A==","X-Gm-Message-State":"AJIora/7ynGvKbd6MIXs1fA7YN41Ig8W40FbZBxaJbugf46eGsCPmkp7\n PXZNujpBT6LyktY6nAZimr6s959jciZhoQ==","X-Google-Smtp-Source":"\n AGRyM1sXe4kYsxC4FJY7QvDSoZJeORcCSvb4dYvG/ZFgoepKsajyELwU8bbwy8GA78G5D+uz03IFiA==","X-Received":"by 2002:a2e:8558:0:b0:25a:a2f3:b69b with SMTP id\n u24-20020a2e8558000000b0025aa2f3b69bmr392833ljj.24.1656467265660;\n Tue, 28 Jun 2022 18:47:45 -0700 (PDT)","Date":"Wed, 29 Jun 2022 04:47:42 +0300","From":"Serge Semin ","To":"Damien Le Moal ","Cc":"Serge Semin ,\n Hans de Goede ,\n Jens Axboe , Hannes Reinecke ,\n Alexey Malahov ,\n Pavel Parkhomenko ,\n Rob Herring , linux-ide@vger.kernel.org,\n linux-kernel@vger.kernel.org, devicetree@vger.kernel.org","Subject":"Re: [PATCH v4 12/23] ata: libahci: Extend port-cmd flags set with\n port capabilities","Message-ID":"<20220629014742.bajkuosd752pg2po@mobilestation>","References":"<20220610081801.11854-1-Sergey.Semin@baikalelectronics.ru>\n <20220610081801.11854-13-Sergey.Semin@baikalelectronics.ru>\n \n <20220615205819.uiqptkqm5qfdvrbj@mobilestation>\n <903e273a-9dc5-f0df-5391-e96e63318323@opensource.wdc.com>\n <20220617203100.jg2o7ponolaenf6r@mobilestation>\n \n <20220618081055.grsrjxa5gqiuhy2i@mobilestation>\n <20220628120820.3nprogcpe2l3bcm4@mobilestation>\n <3dade607-71a5-28ad-fb0b-f5b75c1b5392@opensource.wdc.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<3dade607-71a5-28ad-fb0b-f5b75c1b5392@opensource.wdc.com>","X-Spam-Status":"No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,\n DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,\n RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE\n autolearn=ham autolearn_force=no version=3.4.6","X-Spam-Checker-Version":"SpamAssassin 3.4.6 (2021-04-09) on\n lindbergh.monkeyblade.net","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ide@vger.kernel.org"}}]