[{"id":2912308,"web_url":"http://patchwork.ozlabs.org/comment/2912308/","msgid":"","list_archive_url":null,"date":"2022-06-14T08:42:35","subject":"Re: [PATCH v4 16/23] ata: ahci: Introduce firmware-specific caps\n initialization","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> There are systems with no BIOS or comprehensive embedded firmware which\n> could be able to properly initialize the SATA AHCI controller\n> platform-specific capabilities. In that case a good alternative to having\n> a clever bootloader is to create a device tree node with the properties\n> well describing all the AHCI-related platform specifics. All the settings\n> which are normally detected and marked as available in the HBA and its\n> ports capabilities fields [1] could be defined in the platform DTB by\n> means of a set of the dedicated properties. Such approach perfectly fits\n> to the DTB-philosophy - to provide hardware/platform description.\n> \n> So here we suggest to extend the SATA AHCI device tree bindings with two\n> additional DT-properties:\n> 1) \"hba-cap\" - HBA platform generic capabilities like:\n> - SSS - Staggered Spin-up support.\n> - SMPS - Mechanical Presence Switch support.\n> 2) \"hba-port-cap\" - HBA platform port capabilities like:\n> - HPCP - Hot Plug Capable Port.\n> - MPSP - Mechanical Presence Switch Attached to Port.\n> - CPD - Cold Presence Detection.\n> - ESP - External SATA Port.\n> - FBSCP - FIS-based Switching Capable Port.\n> All of these capabilities require to have a corresponding hardware\n> configuration. Thus it's ok to have them defined in DTB.\n> \n> Even though the driver currently takes into account the state of the ESP\n> and FBSCP flags state only, there is nothing wrong with having all of them\n> supported by the generic AHCI library in order to have a complete OF-based\n> platform-capabilities initialization procedure. These properties will be\n> parsed in the ahci_platform_get_resources() method and their values will\n> be stored in the saved_* fields of the ahci_host_priv structure, which in\n> its turn then will be used to restore the H.CAP, H.PI and P#.CMD\n> capability fields on device init and after HBA reset.\n> \n> Please note this modification concerns the HW-init HBA and its ports flags\n> only, which are by specification [1] are supposed to be initialized by the\n> BIOS/platform firmware/expansion ROM and which are normally declared in\n> the one-time-writable-after-reset register fields. Even though these flags\n> aren't supposed to be cleared after HBA reset some AHCI instances may\n> violate that rule so we still need to perform the fields resetting after\n> each reset. Luckily the corresponding functionality has already been\n> partly implemented in the framework of the ahci_save_initial_config() and\n> ahci_restore_initial_config() methods.\n> \n> [1] Serial ATA AHCI 1.3.1 Specification, p. 103\n> \n> Signed-off-by: Serge Semin \n> \n> ---\n> \n> Changelog v4:\n> - Convert the boolean properties to the bitfield DT-properties. (@Rob)\n> ---\n> drivers/ata/ahci.h | 1 +\n> drivers/ata/libahci.c | 51 ++++++++++++++++++++++++++++------\n> drivers/ata/libahci_platform.c | 41 +++++++++++++++++++++++++--\n> 3 files changed, 82 insertions(+), 11 deletions(-)\n> \n> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h\n> index 8b9826533ae5..0de221055961 100644\n> --- a/drivers/ata/ahci.h\n> +++ b/drivers/ata/ahci.h\n> @@ -337,6 +337,7 @@ struct ahci_host_priv {\n> \tu32\t\t\tsaved_cap;\t/* saved initial cap */\n> \tu32\t\t\tsaved_cap2;\t/* saved initial cap2 */\n> \tu32\t\t\tsaved_port_map;\t/* saved initial port_map */\n> +\tu32\t\t\tsaved_port_cap[AHCI_MAX_PORTS]; /* saved port_cap */\n> \tu32 \t\t\tem_loc; /* enclosure management location */\n> \tu32\t\t\tem_buf_sz;\t/* EM buffer size in byte */\n> \tu32\t\t\tem_msg_type;\t/* EM message type */\n> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c\n> index 1ffaa5f5f21a..954386a2b500 100644\n> --- a/drivers/ata/libahci.c\n> +++ b/drivers/ata/libahci.c\n> @@ -16,6 +16,7 @@\n> * http://www.intel.com/technology/serialata/pdf/rev1_1.pdf\n> */\n> \n> +#include \n> #include \n> #include \n> #include \n> @@ -443,16 +444,28 @@ static ssize_t ahci_show_em_supported(struct device *dev,\n> void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> {\n> \tvoid __iomem *mmio = hpriv->mmio;\n> -\tu32 cap, cap2, vers, port_map;\n> +\tvoid __iomem *port_mmio;\n> +\tunsigned long port_map;\n> +\tu32 cap, cap2, vers;\n> \tint i;\n> \n> \t/* make sure AHCI mode is enabled before accessing CAP */\n> \tahci_enable_ahci(mmio);\n> \n> -\t/* Values prefixed with saved_ are written back to host after\n> -\t * reset. Values without are used for driver operation.\n> +\t/*\n> +\t * Values prefixed with saved_ are written back to the HBA and ports\n> +\t * registers after reset. Values without are used for driver operation.\n> +\t */\n> +\n> +\t/*\n> +\t * Override HW-init HBA capability fields with the platform-specific\n> +\t * values. The rest of the HBA capabilities are defined as Read-only\n> +\t * and can't be modified in CSR anyway.\n> \t */\n> -\thpriv->saved_cap = cap = readl(mmio + HOST_CAP);\n> +\tcap = readl(mmio + HOST_CAP);\n> +\tif (hpriv->saved_cap)\n> +\t\tcap = (cap & ~(HOST_CAP_SSS | HOST_CAP_MPS)) | hpriv->saved_cap;\n> +\thpriv->saved_cap = cap;\n> \n> \t/* CAP2 register is only defined for AHCI 1.2 and later */\n> \tvers = readl(mmio + HOST_VERSION);\n> @@ -519,7 +532,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> \t/* Override the HBA ports mapping if the platform needs it */\n> \tport_map = readl(mmio + HOST_PORTS_IMPL);\n> \tif (hpriv->saved_port_map && port_map != hpriv->saved_port_map) {\n> -\t\tdev_info(dev, \"forcing port_map 0x%x -> 0x%x\\n\",\n> +\t\tdev_info(dev, \"forcing port_map 0x%lx -> 0x%x\\n\",\n\nThis change is not necessary.\n\n> \t\t\t port_map, hpriv->saved_port_map);\n> \t\tport_map = hpriv->saved_port_map;\n> \t} else {\n> @@ -527,7 +540,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> \t}\n> \n> \tif (hpriv->mask_port_map) {\n> -\t\tdev_warn(dev, \"masking port_map 0x%x -> 0x%x\\n\",\n> +\t\tdev_warn(dev, \"masking port_map 0x%lx -> 0x%lx\\n\",\n\nSame.\n\n> \t\t\tport_map,\n> \t\t\tport_map & hpriv->mask_port_map);\n> \t\tport_map &= hpriv->mask_port_map;\n> @@ -546,7 +559,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> \t\t */\n> \t\tif (map_ports > ahci_nr_ports(cap)) {\n> \t\t\tdev_warn(dev,\n> -\t\t\t\t \"implemented port map (0x%x) contains more ports than nr_ports (%u), using nr_ports\\n\",\n> +\t\t\t\t \"implemented port map (0x%lx) contains more ports than nr_ports (%u), using nr_ports\\n\",\n\nSame.\n\n> \t\t\t\t port_map, ahci_nr_ports(cap));\n> \t\t\tport_map = 0;\n> \t\t}\n> @@ -555,12 +568,26 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> \t/* fabricate port_map from cap.nr_ports for < AHCI 1.3 */\n> \tif (!port_map && vers < 0x10300) {\n> \t\tport_map = (1 << ahci_nr_ports(cap)) - 1;\n> -\t\tdev_warn(dev, \"forcing PORTS_IMPL to 0x%x\\n\", port_map);\n> +\t\tdev_warn(dev, \"forcing PORTS_IMPL to 0x%lx\\n\", port_map);\n\nAnd again not needed.\n\n> \n> \t\t/* write the fixed up value to the PI register */\n> \t\thpriv->saved_port_map = port_map;\n> \t}\n> \n> +\t/*\n> +\t * Preserve the ports capabilities defined by the platform. Note there\n> +\t * is no need in storing the rest of the P#.CMD fields since they are\n> +\t * volatile.\n> +\t */\n> +\tfor_each_set_bit(i, &port_map, AHCI_MAX_PORTS) {\n> +\t\tif (hpriv->saved_port_cap[i])\n> +\t\t\tcontinue;\n> +\n> +\t\tport_mmio = __ahci_port_base(hpriv, i);\n> +\t\thpriv->saved_port_cap[i] =\n> +\t\t\treadl(port_mmio + PORT_CMD) & PORT_CMD_CAP;\n> +\t}\n> +\n> \t/* record values to use during operation */\n> \thpriv->cap = cap;\n> \thpriv->cap2 = cap2;\n> @@ -590,13 +617,21 @@ EXPORT_SYMBOL_GPL(ahci_save_initial_config);\n> static void ahci_restore_initial_config(struct ata_host *host)\n> {\n> \tstruct ahci_host_priv *hpriv = host->private_data;\n> +\tunsigned long port_map = hpriv->port_map;\n> \tvoid __iomem *mmio = hpriv->mmio;\n> +\tvoid __iomem *port_mmio;\n> +\tint i;\n> \n> \twritel(hpriv->saved_cap, mmio + HOST_CAP);\n> \tif (hpriv->saved_cap2)\n> \t\twritel(hpriv->saved_cap2, mmio + HOST_CAP2);\n> \twritel(hpriv->saved_port_map, mmio + HOST_PORTS_IMPL);\n> \t(void) readl(mmio + HOST_PORTS_IMPL);\t/* flush */\n> +\n> +\tfor_each_set_bit(i, &port_map, AHCI_MAX_PORTS) {\n> +\t\tport_mmio = __ahci_port_base(hpriv, i);\n> +\t\twritel(hpriv->saved_port_cap[i], port_mmio + PORT_CMD);\n> +\t}\n> }\n> \n> static unsigned ahci_scr_offset(struct ata_port *ap, unsigned int sc_reg)\n> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c\n> index efe640603f3f..8b542a8bc487 100644\n> --- a/drivers/ata/libahci_platform.c\n> +++ b/drivers/ata/libahci_platform.c\n> @@ -23,6 +23,7 @@\n> #include \n> #include \n> #include \n> +\n\nwhite line change.\n\n> #include \"ahci.h\"\n> \n> static void ahci_host_stop(struct ata_host *host);\n> @@ -383,6 +384,34 @@ static int ahci_platform_get_regulator(struct ahci_host_priv *hpriv, u32 port,\n> \treturn rc;\n> }\n> \n> +static int ahci_platform_get_firmware(struct ahci_host_priv *hpriv,\n> +\t\t\t\t struct device *dev)\n> +{\n> +\tstruct device_node *child;\n> +\tu32 port;\n> +\n> +\tif (!of_property_read_u32(dev->of_node, \"hba-cap\", &hpriv->saved_cap))\n> +\t\thpriv->saved_cap &= (HOST_CAP_SSS | HOST_CAP_MPS);\n> +\n> +\tof_property_read_u32(dev->of_node,\n> +\t\t\t \"ports-implemented\", &hpriv->saved_port_map);\n> +\n> +\tfor_each_child_of_node(dev->of_node, child) {\n> +\t\tif (!of_device_is_available(child))\n> +\t\t\tcontinue;\n> +\n> +\t\tif (of_property_read_u32(child, \"reg\", &port)) {\n> +\t\t\tof_node_put(child);\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\n> +\t\tif (!of_property_read_u32(child, \"hba-port-cap\", &hpriv->saved_port_cap[port]))\n> +\t\t\thpriv->saved_port_cap[port] &= PORT_CMD_CAP;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> /**\n> * ahci_platform_get_resources - Get platform resources\n> * @pdev: platform device to get resources for\n> @@ -523,9 +552,6 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,\n> \t\tgoto err_out;\n> \t}\n> \n> -\tof_property_read_u32(dev->of_node,\n> -\t\t\t \"ports-implemented\", &hpriv->saved_port_map);\n> -\n> \tif (child_nodes) {\n> \t\tfor_each_child_of_node(dev->of_node, child) {\n> \t\t\tu32 port;\n> @@ -590,6 +616,15 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,\n> \t\tif (rc == -EPROBE_DEFER)\n> \t\t\tgoto err_out;\n> \t}\n> +\n> +\t/*\n> +\t * Retrieve firmware-specific flags which then will be used to set\n> +\t * the HW-init fields of HBA and its ports\n> +\t */\n> +\trc = ahci_platform_get_firmware(hpriv, dev);\n> +\tif (rc)\n> +\t\tgoto err_out;\n> +\n> \tpm_runtime_enable(dev);\n> \tpm_runtime_get_sync(dev);\n> \thpriv->got_runtime_pm = true;","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=Aw6Emerp;\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=fXeWgrUC;\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 4LMhn93m0yz9sGH\n\tfor ; Tue, 14 Jun 2022 18:44:13 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S1345645AbiFNIoC (ORCPT );\n Tue, 14 Jun 2022 04:44:02 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:42576 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S1355765AbiFNInB (ORCPT\n ); Tue, 14 Jun 2022 04:43:01 -0400","from esa6.hgst.iphmx.com (esa6.hgst.iphmx.com [216.71.154.45])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0435743EDE\n for ; Tue, 14 Jun 2022 01:42:45 -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; 14 Jun 2022 16:42:41 +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 01:01:15 -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:42:41 -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 4LMhlN0JY0z1SVp4\n for ; Tue, 14 Jun 2022 01:42:40 -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 sjBxxy7jJHyH for ;\n Tue, 14 Jun 2022 01:42:38 -0700 (PDT)","from [10.225.163.77] (unknown [10.225.163.77])\n by usg-ed-osssrv.wdc.com (Postfix) with ESMTPSA id 4LMhlJ1qJKz1Rvlc;\n Tue, 14 Jun 2022 01:42:36 -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=1655196166; x=1686732166;\n h=message-id:date:mime-version:subject:to:cc:references:\n from:in-reply-to:content-transfer-encoding;\n bh=seQXUC2yFSK+xh+rRJdR/W+Fvg3IkeD6jADS01tkxBo=;\n b=Aw6EmerpDq8N4764yzJfE6byQP6tokzz/qNNMM2tZYFs6/Ne/MVxmB38\n WizpzZ6YnWkUmu8DNU9ofO231YcecNtS8QS4lz7HWVSdMJ43q4qlGPnJ1\n 9+nd85sYA5LFFoKQ0lUI43+IKJgWujA8gDY99mq3QIbh+fcGdOpSin5fA\n o7Vaoa75XpUcwJENKzF5phA0oHW5xQE5MwQa3FAYFrXDAVwW+6494KlUZ\n hoCd7ZCeUwxKe6rjExdBHK4RorRDJVimHH00FMq5TyDWhPcpkdB5pUkFx\n 5QEMMC5cPUA5US+pk0Qm3lFNLl2l7gYE7rBu0r/Sqtd4b217e1tHhST9z\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 1655196158; x=1657788159; bh=seQXUC2yFSK+xh+rRJdR/W+Fvg3IkeD6jAD\n S01tkxBo=; b=fXeWgrUCqZ+21sntn8IVdwzSKG/Nyml6ACmLw3g+9ilN5+XUUuo\n v3LFhvOMOwTvUd6pKb1mwVFGNYH83Nz63JTrwbeks2iXFX4sJcOapLOy/MMSRVY5\n JrVcl0SQItFUJc1bXxCZjtnd6MwVAntB3rkv+TamljEUZ9RPp3gw/iIl5n+QLluc\n +hF+3fcgECgoVlpwgjkQi50Sp+l2ysrcFXBHHB/zYgeaGvyluEI7HuIYWMJ3A3ez\n wA2WrPWmVAyGqwaaTOTlRdN3vgJUpOwWxLe0ewvhRwFc7l5q9gP9SrO4BFabeWOy\n KV6IHEG94q5p2fpDabloMpWNvkiHrrvs5EA=="],"X-IronPort-AV":"E=Sophos;i=\"5.91,299,1647273600\";\n d=\"scan'208\";a=\"203866929\"","IronPort-SDR":["\n S5XHeP2IINCGnDiqGpDmo6/QkH4HGKhzzGq+oygoNPByCJcdZ7YlAKG284Sl02CBfs5QR0M0f1\n c4EtTqev3rACEgsAWQycZ0T0uT4bXXMsQ2aAE7y4Ovo/XfqUK0SzKUkHZhcztZaGTTrONc5Sbo\n OHqaR5RC+0yIbczUHw4vr83aG4rya5eXjc1n9Tb3IXs4suLvQFeEQX1fkBk/Y/zYfMuvk4b27K\n kcHR+2/YMsRWZyiy2DWC8QV/GtWIDe3/gkTX8sVa9gaVJuzxVPJnas6hYmWqXqiQfIr/e0GjOj\n RsnAN3ZPDrzSS8Smu2ouV2Fx","\n K50MAHiVbwxtHGYTtvZYziOPni2D2SL5QDV+ALtEEHgr7w4LnLhLRTp3B1nnQPEXSJ6oNdGWcP\n IkAIut2U+xKOO+/q5+N/5fvTGrtZueRePU9LCo1cIwHWlToMVp4h2P9Mg8PbajYYNMwDiHUmAN\n 2sh0DWEVhop3SnJlm4gLqGriS8R03A5S24MLQvov76Kdu/bJ+Jx4++WBULpBbtPKgGkyrMWB7y\n uUNxiYVfOpLDPNLhr8M12DL3jKdBQ5b/hAbYPidsEss5U/Wom3wqETZV6aE9vHLIkCc6YWORNl\n 0EY="],"WDCIronportException":"Internal","X-Virus-Scanned":"amavisd-new at usg-ed-osssrv.wdc.com","Message-ID":"","Date":"Tue, 14 Jun 2022 17:42:35 +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 16/23] ata: ahci: Introduce firmware-specific caps\n initialization","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-17-Sergey.Semin@baikalelectronics.ru>","From":"Damien Le Moal ","Organization":"Western Digital Research","In-Reply-To":"<20220610081801.11854-17-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=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":2913739,"web_url":"http://patchwork.ozlabs.org/comment/2913739/","msgid":"<20220615211134.2wxzizbpmfl2akjh@mobilestation>","list_archive_url":null,"date":"2022-06-15T21:11:34","subject":"Re: [PATCH v4 16/23] ata: ahci: Introduce firmware-specific caps\n initialization","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:42:35PM +0900, Damien Le Moal wrote:\n> On 6/10/22 17:17, Serge Semin wrote:\n> > There are systems with no BIOS or comprehensive embedded firmware which\n> > could be able to properly initialize the SATA AHCI controller\n> > platform-specific capabilities. In that case a good alternative to having\n> > a clever bootloader is to create a device tree node with the properties\n> > well describing all the AHCI-related platform specifics. All the settings\n> > which are normally detected and marked as available in the HBA and its\n> > ports capabilities fields [1] could be defined in the platform DTB by\n> > means of a set of the dedicated properties. Such approach perfectly fits\n> > to the DTB-philosophy - to provide hardware/platform description.\n> > \n> > So here we suggest to extend the SATA AHCI device tree bindings with two\n> > additional DT-properties:\n> > 1) \"hba-cap\" - HBA platform generic capabilities like:\n> > - SSS - Staggered Spin-up support.\n> > - SMPS - Mechanical Presence Switch support.\n> > 2) \"hba-port-cap\" - HBA platform port capabilities like:\n> > - HPCP - Hot Plug Capable Port.\n> > - MPSP - Mechanical Presence Switch Attached to Port.\n> > - CPD - Cold Presence Detection.\n> > - ESP - External SATA Port.\n> > - FBSCP - FIS-based Switching Capable Port.\n> > All of these capabilities require to have a corresponding hardware\n> > configuration. Thus it's ok to have them defined in DTB.\n> > \n> > Even though the driver currently takes into account the state of the ESP\n> > and FBSCP flags state only, there is nothing wrong with having all of them\n> > supported by the generic AHCI library in order to have a complete OF-based\n> > platform-capabilities initialization procedure. These properties will be\n> > parsed in the ahci_platform_get_resources() method and their values will\n> > be stored in the saved_* fields of the ahci_host_priv structure, which in\n> > its turn then will be used to restore the H.CAP, H.PI and P#.CMD\n> > capability fields on device init and after HBA reset.\n> > \n> > Please note this modification concerns the HW-init HBA and its ports flags\n> > only, which are by specification [1] are supposed to be initialized by the\n> > BIOS/platform firmware/expansion ROM and which are normally declared in\n> > the one-time-writable-after-reset register fields. Even though these flags\n> > aren't supposed to be cleared after HBA reset some AHCI instances may\n> > violate that rule so we still need to perform the fields resetting after\n> > each reset. Luckily the corresponding functionality has already been\n> > partly implemented in the framework of the ahci_save_initial_config() and\n> > ahci_restore_initial_config() methods.\n> > \n> > [1] Serial ATA AHCI 1.3.1 Specification, p. 103\n> > \n> > Signed-off-by: Serge Semin \n> > \n> > ---\n> > \n> > Changelog v4:\n> > - Convert the boolean properties to the bitfield DT-properties. (@Rob)\n> > ---\n> > drivers/ata/ahci.h | 1 +\n> > drivers/ata/libahci.c | 51 ++++++++++++++++++++++++++++------\n> > drivers/ata/libahci_platform.c | 41 +++++++++++++++++++++++++--\n> > 3 files changed, 82 insertions(+), 11 deletions(-)\n> > \n> > diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h\n> > index 8b9826533ae5..0de221055961 100644\n> > --- a/drivers/ata/ahci.h\n> > +++ b/drivers/ata/ahci.h\n> > @@ -337,6 +337,7 @@ struct ahci_host_priv {\n> > \tu32\t\t\tsaved_cap;\t/* saved initial cap */\n> > \tu32\t\t\tsaved_cap2;\t/* saved initial cap2 */\n> > \tu32\t\t\tsaved_port_map;\t/* saved initial port_map */\n> > +\tu32\t\t\tsaved_port_cap[AHCI_MAX_PORTS]; /* saved port_cap */\n> > \tu32 \t\t\tem_loc; /* enclosure management location */\n> > \tu32\t\t\tem_buf_sz;\t/* EM buffer size in byte */\n> > \tu32\t\t\tem_msg_type;\t/* EM message type */\n> > diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c\n> > index 1ffaa5f5f21a..954386a2b500 100644\n> > --- a/drivers/ata/libahci.c\n> > +++ b/drivers/ata/libahci.c\n> > @@ -16,6 +16,7 @@\n> > * http://www.intel.com/technology/serialata/pdf/rev1_1.pdf\n> > */\n> > \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -443,16 +444,28 @@ static ssize_t ahci_show_em_supported(struct device *dev,\n> > void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> > {\n> > \tvoid __iomem *mmio = hpriv->mmio;\n> > -\tu32 cap, cap2, vers, port_map;\n> > +\tvoid __iomem *port_mmio;\n> > +\tunsigned long port_map;\n> > +\tu32 cap, cap2, vers;\n> > \tint i;\n> > \n> > \t/* make sure AHCI mode is enabled before accessing CAP */\n> > \tahci_enable_ahci(mmio);\n> > \n> > -\t/* Values prefixed with saved_ are written back to host after\n> > -\t * reset. Values without are used for driver operation.\n> > +\t/*\n> > +\t * Values prefixed with saved_ are written back to the HBA and ports\n> > +\t * registers after reset. Values without are used for driver operation.\n> > +\t */\n> > +\n> > +\t/*\n> > +\t * Override HW-init HBA capability fields with the platform-specific\n> > +\t * values. The rest of the HBA capabilities are defined as Read-only\n> > +\t * and can't be modified in CSR anyway.\n> > \t */\n> > -\thpriv->saved_cap = cap = readl(mmio + HOST_CAP);\n> > +\tcap = readl(mmio + HOST_CAP);\n> > +\tif (hpriv->saved_cap)\n> > +\t\tcap = (cap & ~(HOST_CAP_SSS | HOST_CAP_MPS)) | hpriv->saved_cap;\n> > +\thpriv->saved_cap = cap;\n> > \n> > \t/* CAP2 register is only defined for AHCI 1.2 and later */\n> > \tvers = readl(mmio + HOST_VERSION);\n> > @@ -519,7 +532,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> > \t/* Override the HBA ports mapping if the platform needs it */\n> > \tport_map = readl(mmio + HOST_PORTS_IMPL);\n> > \tif (hpriv->saved_port_map && port_map != hpriv->saved_port_map) {\n> > -\t\tdev_info(dev, \"forcing port_map 0x%x -> 0x%x\\n\",\n> > +\t\tdev_info(dev, \"forcing port_map 0x%lx -> 0x%x\\n\",\n> \n\n> This change is not necessary.\n\nIt is. The port_map type has been changed.\n\n> \n> > \t\t\t port_map, hpriv->saved_port_map);\n> > \t\tport_map = hpriv->saved_port_map;\n> > \t} else {\n> > @@ -527,7 +540,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> > \t}\n> > \n> > \tif (hpriv->mask_port_map) {\n> > -\t\tdev_warn(dev, \"masking port_map 0x%x -> 0x%x\\n\",\n> > +\t\tdev_warn(dev, \"masking port_map 0x%lx -> 0x%lx\\n\",\n> \n> Same.\n\nditto\n\n> \n> > \t\t\tport_map,\n> > \t\t\tport_map & hpriv->mask_port_map);\n> > \t\tport_map &= hpriv->mask_port_map;\n> > @@ -546,7 +559,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> > \t\t */\n> > \t\tif (map_ports > ahci_nr_ports(cap)) {\n> > \t\t\tdev_warn(dev,\n> > -\t\t\t\t \"implemented port map (0x%x) contains more ports than nr_ports (%u), using nr_ports\\n\",\n> > +\t\t\t\t \"implemented port map (0x%lx) contains more ports than nr_ports (%u), using nr_ports\\n\",\n> \n> Same.\n\nditto.\n\n> \n> > \t\t\t\t port_map, ahci_nr_ports(cap));\n> > \t\t\tport_map = 0;\n> > \t\t}\n> > @@ -555,12 +568,26 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> > \t/* fabricate port_map from cap.nr_ports for < AHCI 1.3 */\n> > \tif (!port_map && vers < 0x10300) {\n> > \t\tport_map = (1 << ahci_nr_ports(cap)) - 1;\n> > -\t\tdev_warn(dev, \"forcing PORTS_IMPL to 0x%x\\n\", port_map);\n> > +\t\tdev_warn(dev, \"forcing PORTS_IMPL to 0x%lx\\n\", port_map);\n> \n> And again not needed.\n\nand ditto.\n\n> \n> > \n> > \t\t/* write the fixed up value to the PI register */\n> > \t\thpriv->saved_port_map = port_map;\n> > \t}\n> > \n> > +\t/*\n> > +\t * Preserve the ports capabilities defined by the platform. Note there\n> > +\t * is no need in storing the rest of the P#.CMD fields since they are\n> > +\t * volatile.\n> > +\t */\n> > +\tfor_each_set_bit(i, &port_map, AHCI_MAX_PORTS) {\n> > +\t\tif (hpriv->saved_port_cap[i])\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tport_mmio = __ahci_port_base(hpriv, i);\n> > +\t\thpriv->saved_port_cap[i] =\n> > +\t\t\treadl(port_mmio + PORT_CMD) & PORT_CMD_CAP;\n> > +\t}\n> > +\n> > \t/* record values to use during operation */\n> > \thpriv->cap = cap;\n> > \thpriv->cap2 = cap2;\n> > @@ -590,13 +617,21 @@ EXPORT_SYMBOL_GPL(ahci_save_initial_config);\n> > static void ahci_restore_initial_config(struct ata_host *host)\n> > {\n> > \tstruct ahci_host_priv *hpriv = host->private_data;\n> > +\tunsigned long port_map = hpriv->port_map;\n> > \tvoid __iomem *mmio = hpriv->mmio;\n> > +\tvoid __iomem *port_mmio;\n> > +\tint i;\n> > \n> > \twritel(hpriv->saved_cap, mmio + HOST_CAP);\n> > \tif (hpriv->saved_cap2)\n> > \t\twritel(hpriv->saved_cap2, mmio + HOST_CAP2);\n> > \twritel(hpriv->saved_port_map, mmio + HOST_PORTS_IMPL);\n> > \t(void) readl(mmio + HOST_PORTS_IMPL);\t/* flush */\n> > +\n> > +\tfor_each_set_bit(i, &port_map, AHCI_MAX_PORTS) {\n> > +\t\tport_mmio = __ahci_port_base(hpriv, i);\n> > +\t\twritel(hpriv->saved_port_cap[i], port_mmio + PORT_CMD);\n> > +\t}\n> > }\n> > \n> > static unsigned ahci_scr_offset(struct ata_port *ap, unsigned int sc_reg)\n> > diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c\n> > index efe640603f3f..8b542a8bc487 100644\n> > --- a/drivers/ata/libahci_platform.c\n> > +++ b/drivers/ata/libahci_platform.c\n> > @@ -23,6 +23,7 @@\n> > #include \n> > #include \n> > #include \n> > +\n> \n> white line change.\n\nOk. I'll drop it.\n\n-Sergey\n\n> \n> > #include \"ahci.h\"\n> > \n> > static void ahci_host_stop(struct ata_host *host);\n> > @@ -383,6 +384,34 @@ static int ahci_platform_get_regulator(struct ahci_host_priv *hpriv, u32 port,\n> > \treturn rc;\n> > }\n> > \n> > +static int ahci_platform_get_firmware(struct ahci_host_priv *hpriv,\n> > +\t\t\t\t struct device *dev)\n> > +{\n> > +\tstruct device_node *child;\n> > +\tu32 port;\n> > +\n> > +\tif (!of_property_read_u32(dev->of_node, \"hba-cap\", &hpriv->saved_cap))\n> > +\t\thpriv->saved_cap &= (HOST_CAP_SSS | HOST_CAP_MPS);\n> > +\n> > +\tof_property_read_u32(dev->of_node,\n> > +\t\t\t \"ports-implemented\", &hpriv->saved_port_map);\n> > +\n> > +\tfor_each_child_of_node(dev->of_node, child) {\n> > +\t\tif (!of_device_is_available(child))\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tif (of_property_read_u32(child, \"reg\", &port)) {\n> > +\t\t\tof_node_put(child);\n> > +\t\t\treturn -EINVAL;\n> > +\t\t}\n> > +\n> > +\t\tif (!of_property_read_u32(child, \"hba-port-cap\", &hpriv->saved_port_cap[port]))\n> > +\t\t\thpriv->saved_port_cap[port] &= PORT_CMD_CAP;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > /**\n> > * ahci_platform_get_resources - Get platform resources\n> > * @pdev: platform device to get resources for\n> > @@ -523,9 +552,6 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,\n> > \t\tgoto err_out;\n> > \t}\n> > \n> > -\tof_property_read_u32(dev->of_node,\n> > -\t\t\t \"ports-implemented\", &hpriv->saved_port_map);\n> > -\n> > \tif (child_nodes) {\n> > \t\tfor_each_child_of_node(dev->of_node, child) {\n> > \t\t\tu32 port;\n> > @@ -590,6 +616,15 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,\n> > \t\tif (rc == -EPROBE_DEFER)\n> > \t\t\tgoto err_out;\n> > \t}\n> > +\n> > +\t/*\n> > +\t * Retrieve firmware-specific flags which then will be used to set\n> > +\t * the HW-init fields of HBA and its ports\n> > +\t */\n> > +\trc = ahci_platform_get_firmware(hpriv, dev);\n> > +\tif (rc)\n> > +\t\tgoto err_out;\n> > +\n> > \tpm_runtime_enable(dev);\n> > \tpm_runtime_get_sync(dev);\n> > \thpriv->got_runtime_pm = true;\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=fQcCLxVP;\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 4LNdKD22rPz9sFx\n\tfor ; Thu, 16 Jun 2022 07:11:44 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S235568AbiFOVLk (ORCPT );\n Wed, 15 Jun 2022 17:11:40 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:36730 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S231149AbiFOVLk (ORCPT\n ); Wed, 15 Jun 2022 17:11:40 -0400","from mail-lj1-x236.google.com (mail-lj1-x236.google.com\n [IPv6:2a00:1450:4864:20::236])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D061134662;\n Wed, 15 Jun 2022 14:11:38 -0700 (PDT)","by mail-lj1-x236.google.com with SMTP id l20so7905791lji.0;\n Wed, 15 Jun 2022 14:11:38 -0700 (PDT)","from mobilestation ([95.79.189.214])\n by smtp.gmail.com with ESMTPSA id\n k17-20020a05651210d100b00478c1ac6d98sm1919742lfg.248.2022.06.15.14.11.35\n (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n Wed, 15 Jun 2022 14:11:36 -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=SDJiQxzZpxq9bSwCVPiMiIqaiBt6aGZyMnOeorPzHy8=;\n b=fQcCLxVPiUeLxQ+P5DCjILqCA6EtwRNy4Gp40vQLTHudPViuUAGAZG8AZ6hUNXth+1\n drx2xXmoVX01hzGJ7oIEviweADAEg0wctetDZQRQdgM+UEhP3Jd6Pkk0GKV+PJsC/gbf\n dV0az/t/kn8f8QJmBJgDC+4YsmjuEfdbxYDP09dMZcM7QQKgkJU5zipjyyXWIz8d5ITZ\n +idQBwlN2TKjJ+WliecEVbaGUwzcDh1OX1k31feXYKupY4/L7wwigvZ+8KfKediYCRWd\n YvC2unGyDTr9EyrYY1ZN1+PzW80yHOrOrZPUuPmMiiea3pp45yHBFO4I9sbNtASR5h/y\n PXhg==","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=SDJiQxzZpxq9bSwCVPiMiIqaiBt6aGZyMnOeorPzHy8=;\n b=CWKYz5oPrc1x3RFnbbrxrWP5wHS+rwskEsGCxuwG1uEJQxeslPvemLFuJ0DssylIk3\n 7SqCcSGuD1n338ncrmzc7TrxWSn7BRaY8iaVvIBKGXxadFn/zOu0EwPZqzAfH14xBg9A\n c5ClDr5EPmVoQjCyO+crHsnm2AwaPRS4C/w0cu9AQgeJkCPoe/ktb5Hco98xfQfZ2FBe\n 1CHKw8WayFEkfDp/nQXcO5eL8XTaUuAdo7kvl+Xw/0ZKjYqsMotNwu1xI3tF4ykkEMJz\n xsKdhrlrRlBg1Rga5t4fH2dbJDrKZEP+c9psy48A/3KRI8VmrEUYLwnCTnNudmxsKRdS\n w9iw==","X-Gm-Message-State":"AJIora8X29OduhYRd2O9h3Gf6rp5NNItyKxkjJkAinVj+qrYuS7fPbYz\n fH6o1Cp8Nu6xAJZU24+vq3s=","X-Google-Smtp-Source":"\n AGRyM1url30wW5I4PO3+Jjf1+oBcXXVYsE03okdtZmoEYEC0yHI4OBvc2DSl6bRal8o76Fr8kmJVDQ==","X-Received":"by 2002:a2e:a40c:0:b0:255:8b11:88d8 with SMTP id\n p12-20020a2ea40c000000b002558b1188d8mr857591ljn.222.1655327497121;\n Wed, 15 Jun 2022 14:11:37 -0700 (PDT)","Date":"Thu, 16 Jun 2022 00:11:34 +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 16/23] ata: ahci: Introduce firmware-specific caps\n initialization","Message-ID":"<20220615211134.2wxzizbpmfl2akjh@mobilestation>","References":"<20220610081801.11854-1-Sergey.Semin@baikalelectronics.ru>\n <20220610081801.11854-17-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":2913854,"web_url":"http://patchwork.ozlabs.org/comment/2913854/","msgid":"","list_archive_url":null,"date":"2022-06-16T00:29:50","subject":"Re: [PATCH v4 16/23] ata: ahci: Introduce firmware-specific caps\n initialization","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 6:11, Serge Semin wrote:\n> On Tue, Jun 14, 2022 at 05:42:35PM +0900, Damien Le Moal wrote:\n>> On 6/10/22 17:17, Serge Semin wrote:\n>>> There are systems with no BIOS or comprehensive embedded firmware which\n>>> could be able to properly initialize the SATA AHCI controller\n>>> platform-specific capabilities. In that case a good alternative to having\n>>> a clever bootloader is to create a device tree node with the properties\n>>> well describing all the AHCI-related platform specifics. All the settings\n>>> which are normally detected and marked as available in the HBA and its\n>>> ports capabilities fields [1] could be defined in the platform DTB by\n>>> means of a set of the dedicated properties. Such approach perfectly fits\n>>> to the DTB-philosophy - to provide hardware/platform description.\n>>>\n>>> So here we suggest to extend the SATA AHCI device tree bindings with two\n>>> additional DT-properties:\n>>> 1) \"hba-cap\" - HBA platform generic capabilities like:\n>>> - SSS - Staggered Spin-up support.\n>>> - SMPS - Mechanical Presence Switch support.\n>>> 2) \"hba-port-cap\" - HBA platform port capabilities like:\n>>> - HPCP - Hot Plug Capable Port.\n>>> - MPSP - Mechanical Presence Switch Attached to Port.\n>>> - CPD - Cold Presence Detection.\n>>> - ESP - External SATA Port.\n>>> - FBSCP - FIS-based Switching Capable Port.\n>>> All of these capabilities require to have a corresponding hardware\n>>> configuration. Thus it's ok to have them defined in DTB.\n>>>\n>>> Even though the driver currently takes into account the state of the ESP\n>>> and FBSCP flags state only, there is nothing wrong with having all of them\n>>> supported by the generic AHCI library in order to have a complete OF-based\n>>> platform-capabilities initialization procedure. These properties will be\n>>> parsed in the ahci_platform_get_resources() method and their values will\n>>> be stored in the saved_* fields of the ahci_host_priv structure, which in\n>>> its turn then will be used to restore the H.CAP, H.PI and P#.CMD\n>>> capability fields on device init and after HBA reset.\n>>>\n>>> Please note this modification concerns the HW-init HBA and its ports flags\n>>> only, which are by specification [1] are supposed to be initialized by the\n>>> BIOS/platform firmware/expansion ROM and which are normally declared in\n>>> the one-time-writable-after-reset register fields. Even though these flags\n>>> aren't supposed to be cleared after HBA reset some AHCI instances may\n>>> violate that rule so we still need to perform the fields resetting after\n>>> each reset. Luckily the corresponding functionality has already been\n>>> partly implemented in the framework of the ahci_save_initial_config() and\n>>> ahci_restore_initial_config() methods.\n>>>\n>>> [1] Serial ATA AHCI 1.3.1 Specification, p. 103\n>>>\n>>> Signed-off-by: Serge Semin \n>>>\n>>> ---\n>>>\n>>> Changelog v4:\n>>> - Convert the boolean properties to the bitfield DT-properties. (@Rob)\n>>> ---\n>>> drivers/ata/ahci.h | 1 +\n>>> drivers/ata/libahci.c | 51 ++++++++++++++++++++++++++++------\n>>> drivers/ata/libahci_platform.c | 41 +++++++++++++++++++++++++--\n>>> 3 files changed, 82 insertions(+), 11 deletions(-)\n>>>\n>>> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h\n>>> index 8b9826533ae5..0de221055961 100644\n>>> --- a/drivers/ata/ahci.h\n>>> +++ b/drivers/ata/ahci.h\n>>> @@ -337,6 +337,7 @@ struct ahci_host_priv {\n>>> \tu32\t\t\tsaved_cap;\t/* saved initial cap */\n>>> \tu32\t\t\tsaved_cap2;\t/* saved initial cap2 */\n>>> \tu32\t\t\tsaved_port_map;\t/* saved initial port_map */\n>>> +\tu32\t\t\tsaved_port_cap[AHCI_MAX_PORTS]; /* saved port_cap */\n>>> \tu32 \t\t\tem_loc; /* enclosure management location */\n>>> \tu32\t\t\tem_buf_sz;\t/* EM buffer size in byte */\n>>> \tu32\t\t\tem_msg_type;\t/* EM message type */\n>>> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c\n>>> index 1ffaa5f5f21a..954386a2b500 100644\n>>> --- a/drivers/ata/libahci.c\n>>> +++ b/drivers/ata/libahci.c\n>>> @@ -16,6 +16,7 @@\n>>> * http://www.intel.com/technology/serialata/pdf/rev1_1.pdf\n>>> */\n>>> \n>>> +#include \n>>> #include \n>>> #include \n>>> #include \n>>> @@ -443,16 +444,28 @@ static ssize_t ahci_show_em_supported(struct device *dev,\n>>> void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n>>> {\n>>> \tvoid __iomem *mmio = hpriv->mmio;\n>>> -\tu32 cap, cap2, vers, port_map;\n>>> +\tvoid __iomem *port_mmio;\n>>> +\tunsigned long port_map;\n>>> +\tu32 cap, cap2, vers;\n>>> \tint i;\n>>> \n>>> \t/* make sure AHCI mode is enabled before accessing CAP */\n>>> \tahci_enable_ahci(mmio);\n>>> \n>>> -\t/* Values prefixed with saved_ are written back to host after\n>>> -\t * reset. Values without are used for driver operation.\n>>> +\t/*\n>>> +\t * Values prefixed with saved_ are written back to the HBA and ports\n>>> +\t * registers after reset. Values without are used for driver operation.\n>>> +\t */\n>>> +\n>>> +\t/*\n>>> +\t * Override HW-init HBA capability fields with the platform-specific\n>>> +\t * values. The rest of the HBA capabilities are defined as Read-only\n>>> +\t * and can't be modified in CSR anyway.\n>>> \t */\n>>> -\thpriv->saved_cap = cap = readl(mmio + HOST_CAP);\n>>> +\tcap = readl(mmio + HOST_CAP);\n>>> +\tif (hpriv->saved_cap)\n>>> +\t\tcap = (cap & ~(HOST_CAP_SSS | HOST_CAP_MPS)) | hpriv->saved_cap;\n>>> +\thpriv->saved_cap = cap;\n>>> \n>>> \t/* CAP2 register is only defined for AHCI 1.2 and later */\n>>> \tvers = readl(mmio + HOST_VERSION);\n>>> @@ -519,7 +532,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n>>> \t/* Override the HBA ports mapping if the platform needs it */\n>>> \tport_map = readl(mmio + HOST_PORTS_IMPL);\n>>> \tif (hpriv->saved_port_map && port_map != hpriv->saved_port_map) {\n>>> -\t\tdev_info(dev, \"forcing port_map 0x%x -> 0x%x\\n\",\n>>> +\t\tdev_info(dev, \"forcing port_map 0x%lx -> 0x%x\\n\",\n>>\n> \n>> This change is not necessary.\n> \n> It is. The port_map type has been changed.\n\nIgnore. When I read the patches the other day, the mailer font had that \"l\" look\nlike a \"1\" :) My mistake.\n\n> \n>>\n>>> \t\t\t port_map, hpriv->saved_port_map);\n>>> \t\tport_map = hpriv->saved_port_map;\n>>> \t} else {\n>>> @@ -527,7 +540,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n>>> \t}\n>>> \n>>> \tif (hpriv->mask_port_map) {\n>>> -\t\tdev_warn(dev, \"masking port_map 0x%x -> 0x%x\\n\",\n>>> +\t\tdev_warn(dev, \"masking port_map 0x%lx -> 0x%lx\\n\",\n>>\n>> Same.\n> \n> ditto\n> \n>>\n>>> \t\t\tport_map,\n>>> \t\t\tport_map & hpriv->mask_port_map);\n>>> \t\tport_map &= hpriv->mask_port_map;\n>>> @@ -546,7 +559,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n>>> \t\t */\n>>> \t\tif (map_ports > ahci_nr_ports(cap)) {\n>>> \t\t\tdev_warn(dev,\n>>> -\t\t\t\t \"implemented port map (0x%x) contains more ports than nr_ports (%u), using nr_ports\\n\",\n>>> +\t\t\t\t \"implemented port map (0x%lx) contains more ports than nr_ports (%u), using nr_ports\\n\",\n>>\n>> Same.\n> \n> ditto.\n> \n>>\n>>> \t\t\t\t port_map, ahci_nr_ports(cap));\n>>> \t\t\tport_map = 0;\n>>> \t\t}\n>>> @@ -555,12 +568,26 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n>>> \t/* fabricate port_map from cap.nr_ports for < AHCI 1.3 */\n>>> \tif (!port_map && vers < 0x10300) {\n>>> \t\tport_map = (1 << ahci_nr_ports(cap)) - 1;\n>>> -\t\tdev_warn(dev, \"forcing PORTS_IMPL to 0x%x\\n\", port_map);\n>>> +\t\tdev_warn(dev, \"forcing PORTS_IMPL to 0x%lx\\n\", port_map);\n>>\n>> And again not needed.\n> \n> and ditto.\n> \n>>\n>>> \n>>> \t\t/* write the fixed up value to the PI register */\n>>> \t\thpriv->saved_port_map = port_map;\n>>> \t}\n>>> \n>>> +\t/*\n>>> +\t * Preserve the ports capabilities defined by the platform. Note there\n>>> +\t * is no need in storing the rest of the P#.CMD fields since they are\n>>> +\t * volatile.\n>>> +\t */\n>>> +\tfor_each_set_bit(i, &port_map, AHCI_MAX_PORTS) {\n>>> +\t\tif (hpriv->saved_port_cap[i])\n>>> +\t\t\tcontinue;\n>>> +\n>>> +\t\tport_mmio = __ahci_port_base(hpriv, i);\n>>> +\t\thpriv->saved_port_cap[i] =\n>>> +\t\t\treadl(port_mmio + PORT_CMD) & PORT_CMD_CAP;\n>>> +\t}\n>>> +\n>>> \t/* record values to use during operation */\n>>> \thpriv->cap = cap;\n>>> \thpriv->cap2 = cap2;\n>>> @@ -590,13 +617,21 @@ EXPORT_SYMBOL_GPL(ahci_save_initial_config);\n>>> static void ahci_restore_initial_config(struct ata_host *host)\n>>> {\n>>> \tstruct ahci_host_priv *hpriv = host->private_data;\n>>> +\tunsigned long port_map = hpriv->port_map;\n>>> \tvoid __iomem *mmio = hpriv->mmio;\n>>> +\tvoid __iomem *port_mmio;\n>>> +\tint i;\n>>> \n>>> \twritel(hpriv->saved_cap, mmio + HOST_CAP);\n>>> \tif (hpriv->saved_cap2)\n>>> \t\twritel(hpriv->saved_cap2, mmio + HOST_CAP2);\n>>> \twritel(hpriv->saved_port_map, mmio + HOST_PORTS_IMPL);\n>>> \t(void) readl(mmio + HOST_PORTS_IMPL);\t/* flush */\n>>> +\n>>> +\tfor_each_set_bit(i, &port_map, AHCI_MAX_PORTS) {\n>>> +\t\tport_mmio = __ahci_port_base(hpriv, i);\n>>> +\t\twritel(hpriv->saved_port_cap[i], port_mmio + PORT_CMD);\n>>> +\t}\n>>> }\n>>> \n>>> static unsigned ahci_scr_offset(struct ata_port *ap, unsigned int sc_reg)\n>>> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c\n>>> index efe640603f3f..8b542a8bc487 100644\n>>> --- a/drivers/ata/libahci_platform.c\n>>> +++ b/drivers/ata/libahci_platform.c\n>>> @@ -23,6 +23,7 @@\n>>> #include \n>>> #include \n>>> #include \n>>> +\n>>\n>> white line change.\n> \n> Ok. I'll drop it.\n> \n> -Sergey\n> \n>>\n>>> #include \"ahci.h\"\n>>> \n>>> static void ahci_host_stop(struct ata_host *host);\n>>> @@ -383,6 +384,34 @@ static int ahci_platform_get_regulator(struct ahci_host_priv *hpriv, u32 port,\n>>> \treturn rc;\n>>> }\n>>> \n>>> +static int ahci_platform_get_firmware(struct ahci_host_priv *hpriv,\n>>> +\t\t\t\t struct device *dev)\n>>> +{\n>>> +\tstruct device_node *child;\n>>> +\tu32 port;\n>>> +\n>>> +\tif (!of_property_read_u32(dev->of_node, \"hba-cap\", &hpriv->saved_cap))\n>>> +\t\thpriv->saved_cap &= (HOST_CAP_SSS | HOST_CAP_MPS);\n>>> +\n>>> +\tof_property_read_u32(dev->of_node,\n>>> +\t\t\t \"ports-implemented\", &hpriv->saved_port_map);\n>>> +\n>>> +\tfor_each_child_of_node(dev->of_node, child) {\n>>> +\t\tif (!of_device_is_available(child))\n>>> +\t\t\tcontinue;\n>>> +\n>>> +\t\tif (of_property_read_u32(child, \"reg\", &port)) {\n>>> +\t\t\tof_node_put(child);\n>>> +\t\t\treturn -EINVAL;\n>>> +\t\t}\n>>> +\n>>> +\t\tif (!of_property_read_u32(child, \"hba-port-cap\", &hpriv->saved_port_cap[port]))\n>>> +\t\t\thpriv->saved_port_cap[port] &= PORT_CMD_CAP;\n>>> +\t}\n>>> +\n>>> +\treturn 0;\n>>> +}\n>>> +\n>>> /**\n>>> * ahci_platform_get_resources - Get platform resources\n>>> * @pdev: platform device to get resources for\n>>> @@ -523,9 +552,6 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,\n>>> \t\tgoto err_out;\n>>> \t}\n>>> \n>>> -\tof_property_read_u32(dev->of_node,\n>>> -\t\t\t \"ports-implemented\", &hpriv->saved_port_map);\n>>> -\n>>> \tif (child_nodes) {\n>>> \t\tfor_each_child_of_node(dev->of_node, child) {\n>>> \t\t\tu32 port;\n>>> @@ -590,6 +616,15 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,\n>>> \t\tif (rc == -EPROBE_DEFER)\n>>> \t\t\tgoto err_out;\n>>> \t}\n>>> +\n>>> +\t/*\n>>> +\t * Retrieve firmware-specific flags which then will be used to set\n>>> +\t * the HW-init fields of HBA and its ports\n>>> +\t */\n>>> +\trc = ahci_platform_get_firmware(hpriv, dev);\n>>> +\tif (rc)\n>>> +\t\tgoto err_out;\n>>> +\n>>> \tpm_runtime_enable(dev);\n>>> \tpm_runtime_get_sync(dev);\n>>> \thpriv->got_runtime_pm = true;\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=AxL4PH7t;\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=WBsbuh5p;\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 4LNjjz2JTxz9s2R\n\tfor ; Thu, 16 Jun 2022 10:29:59 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S1343887AbiFPA36 (ORCPT );\n Wed, 15 Jun 2022 20:29:58 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:37678 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S242636AbiFPA35 (ORCPT\n ); Wed, 15 Jun 2022 20:29:57 -0400","from esa6.hgst.iphmx.com (esa6.hgst.iphmx.com [216.71.154.45])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D7F2656FBB\n for ; Wed, 15 Jun 2022 17:29:55 -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; 16 Jun 2022 08:29:56 +0800","from uls-op-cesaip01.wdc.com ([10.248.3.36])\n by uls-op-cesaep01.wdc.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256;\n 15 Jun 2022 16:52:45 -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:29:55 -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 4LNjjv02Y5z1SHwl\n for ; Wed, 15 Jun 2022 17:29:54 -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 8DqIeXJVbe7h for ;\n Wed, 15 Jun 2022 17:29:53 -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 4LNjjq3Nwrz1Rvlc;\n Wed, 15 Jun 2022 17:29:51 -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=1655339396; x=1686875396;\n h=message-id:date:mime-version:subject:to:cc:references:\n from:in-reply-to:content-transfer-encoding;\n bh=UpXQHU0gqujhxD5jb4VyE96JVSMoT9SxWll6RvwlrfQ=;\n b=AxL4PH7tAnJGGTJXO4bRuIIN7ZqjgJtyhRf6xA9dXK/3IFiUqixoY2T7\n Zs16aRuZDhmfVRG4+CN7BkXnWAvLOtanWkv3uu4hk/RCIiz3EbX+EnMuS\n DbYWH1k1QfWnS+EZpZrJSyU87sp0mynzDDEg+3RmCmzgFa1tgyzUPIUix\n 0SbW0i+4lFJlaLB97EUeSqHvkQaMxMbCtJy1DmlxdesjjiZzWSPf82N96\n ZNYKjDpMlE4Lbp+u7McLCMu+tmHxlLS09lkL/sMJN/BAM1el/y5d+QN1W\n uLoWghZI3hUv6C9zfuXf2Y4AeNdPx+KfWj7TI0xF2eY8JrEpku2dRzs5t\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 1655339393; x=1657931394; bh=UpXQHU0gqujhxD5jb4VyE96JVSMoT9SxWll\n 6RvwlrfQ=; b=WBsbuh5pNKtQzyheI0o8mg1Wjt5MDWZFWHDPy8QXbwzUi7OYH5w\n zJUut5VzSAVDq9W4pT18POcb1rJ38D4KB2OLEJeRiU+f37w5rmeVvJsun9tG0Ln5\n SzYTcGunt8PxOpX+EOqZ6c8h/ulamML2DDch9wafUJmpQuDDZQPzWgD4FI5DeZpx\n vsIVJUM7YTxsDzdPLnkIw8zTOlmqAuIRnPu1ycBQtRxG5ogy1QWBAc8JvVrsEp9p\n NDvjeCFXQU24PMZcIhNsUcVvKI+lAiQUsfo1FcsIsdYSH8st/HUQX4sRz6HzTNy3\n /ynYQioNChffK5glDccWDpXJ4hRGeOSW09w=="],"X-IronPort-AV":"E=Sophos;i=\"5.91,302,1647273600\";\n d=\"scan'208\";a=\"204038353\"","IronPort-SDR":["\n ADwlIf5RXvatK9f2IGgEi74xBNPkGeVH0l34VFYNNXhby77y4xe+u1hCVZLJUJNohwfj026VfC\n 1wcYRv+MOTDLkFv7URURJc3d/VTMA9J6LUumunz1hxs8mud/vM2+nPO9FJmiNWZNo9wZDUXEwz\n bBUr1sr+JQeFDj03/GYO9QNqiI2iWL9PxPtqBENhtSpH1oOjAxJJ0aSUC/oyRoyTFUqbnQBrlk\n 234x175qV4y1GqGXkKwH8dAt7P4+aMEfTuR4PcRS80tZDqEdRExHlv4mAkZIuO195wPsu0jatE\n pnUzwbg63WhDpKaSU5449lzt","\n ofAuF27eOD10T0MB4ZKQZs9Inpzj39XlZLIsIVFqUDnIiDAqFb4neLC6QvI0ZcQJ93ZxY1uHEm\n hDPvFONJqqeIPvnKMex68orBD6JCWacbg51gKvv1dN3sUQsne8ZbSkGN6HwrxcNLELtRqNaSfW\n Ozwz0naqXTvC9KIqo0hjiAt6NlKf5J4xcV0MsZOmMsZGsuX9vzk+aNeid7aw7R+Ye/40Q283hQ\n sTjMQiAqIvP6zZ9EUHSCe6RMb6riyuIetJ4D+bc94bdR0boVBJBvq719SgO5QwC5p3XP8IfwRC\n Uw4="],"WDCIronportException":"Internal","X-Virus-Scanned":"amavisd-new at usg-ed-osssrv.wdc.com","Message-ID":"","Date":"Thu, 16 Jun 2022 09:29:50 +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 16/23] ata: ahci: Introduce firmware-specific caps\n initialization","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-17-Sergey.Semin@baikalelectronics.ru>\n \n <20220615211134.2wxzizbpmfl2akjh@mobilestation>","From":"Damien Le Moal ","Organization":"Western Digital Research","In-Reply-To":"<20220615211134.2wxzizbpmfl2akjh@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=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":2915222,"web_url":"http://patchwork.ozlabs.org/comment/2915222/","msgid":"<20220617203242.3ujyknllrx2frzmq@mobilestation>","list_archive_url":null,"date":"2022-06-17T20:32:42","subject":"Re: [PATCH v4 16/23] ata: ahci: Introduce firmware-specific caps\n initialization","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:29:50AM +0900, Damien Le Moal wrote:\n> On 2022/06/16 6:11, Serge Semin wrote:\n> > On Tue, Jun 14, 2022 at 05:42:35PM +0900, Damien Le Moal wrote:\n> >> On 6/10/22 17:17, Serge Semin wrote:\n> >>> There are systems with no BIOS or comprehensive embedded firmware which\n> >>> could be able to properly initialize the SATA AHCI controller\n> >>> platform-specific capabilities. In that case a good alternative to having\n> >>> a clever bootloader is to create a device tree node with the properties\n> >>> well describing all the AHCI-related platform specifics. All the settings\n> >>> which are normally detected and marked as available in the HBA and its\n> >>> ports capabilities fields [1] could be defined in the platform DTB by\n> >>> means of a set of the dedicated properties. Such approach perfectly fits\n> >>> to the DTB-philosophy - to provide hardware/platform description.\n> >>>\n> >>> So here we suggest to extend the SATA AHCI device tree bindings with two\n> >>> additional DT-properties:\n> >>> 1) \"hba-cap\" - HBA platform generic capabilities like:\n> >>> - SSS - Staggered Spin-up support.\n> >>> - SMPS - Mechanical Presence Switch support.\n> >>> 2) \"hba-port-cap\" - HBA platform port capabilities like:\n> >>> - HPCP - Hot Plug Capable Port.\n> >>> - MPSP - Mechanical Presence Switch Attached to Port.\n> >>> - CPD - Cold Presence Detection.\n> >>> - ESP - External SATA Port.\n> >>> - FBSCP - FIS-based Switching Capable Port.\n> >>> All of these capabilities require to have a corresponding hardware\n> >>> configuration. Thus it's ok to have them defined in DTB.\n> >>>\n> >>> Even though the driver currently takes into account the state of the ESP\n> >>> and FBSCP flags state only, there is nothing wrong with having all of them\n> >>> supported by the generic AHCI library in order to have a complete OF-based\n> >>> platform-capabilities initialization procedure. These properties will be\n> >>> parsed in the ahci_platform_get_resources() method and their values will\n> >>> be stored in the saved_* fields of the ahci_host_priv structure, which in\n> >>> its turn then will be used to restore the H.CAP, H.PI and P#.CMD\n> >>> capability fields on device init and after HBA reset.\n> >>>\n> >>> Please note this modification concerns the HW-init HBA and its ports flags\n> >>> only, which are by specification [1] are supposed to be initialized by the\n> >>> BIOS/platform firmware/expansion ROM and which are normally declared in\n> >>> the one-time-writable-after-reset register fields. Even though these flags\n> >>> aren't supposed to be cleared after HBA reset some AHCI instances may\n> >>> violate that rule so we still need to perform the fields resetting after\n> >>> each reset. Luckily the corresponding functionality has already been\n> >>> partly implemented in the framework of the ahci_save_initial_config() and\n> >>> ahci_restore_initial_config() methods.\n> >>>\n> >>> [1] Serial ATA AHCI 1.3.1 Specification, p. 103\n> >>>\n> >>> Signed-off-by: Serge Semin \n> >>>\n> >>> ---\n> >>>\n> >>> Changelog v4:\n> >>> - Convert the boolean properties to the bitfield DT-properties. (@Rob)\n> >>> ---\n> >>> drivers/ata/ahci.h | 1 +\n> >>> drivers/ata/libahci.c | 51 ++++++++++++++++++++++++++++------\n> >>> drivers/ata/libahci_platform.c | 41 +++++++++++++++++++++++++--\n> >>> 3 files changed, 82 insertions(+), 11 deletions(-)\n> >>>\n> >>> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h\n> >>> index 8b9826533ae5..0de221055961 100644\n> >>> --- a/drivers/ata/ahci.h\n> >>> +++ b/drivers/ata/ahci.h\n> >>> @@ -337,6 +337,7 @@ struct ahci_host_priv {\n> >>> \tu32\t\t\tsaved_cap;\t/* saved initial cap */\n> >>> \tu32\t\t\tsaved_cap2;\t/* saved initial cap2 */\n> >>> \tu32\t\t\tsaved_port_map;\t/* saved initial port_map */\n> >>> +\tu32\t\t\tsaved_port_cap[AHCI_MAX_PORTS]; /* saved port_cap */\n> >>> \tu32 \t\t\tem_loc; /* enclosure management location */\n> >>> \tu32\t\t\tem_buf_sz;\t/* EM buffer size in byte */\n> >>> \tu32\t\t\tem_msg_type;\t/* EM message type */\n> >>> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c\n> >>> index 1ffaa5f5f21a..954386a2b500 100644\n> >>> --- a/drivers/ata/libahci.c\n> >>> +++ b/drivers/ata/libahci.c\n> >>> @@ -16,6 +16,7 @@\n> >>> * http://www.intel.com/technology/serialata/pdf/rev1_1.pdf\n> >>> */\n> >>> \n> >>> +#include \n> >>> #include \n> >>> #include \n> >>> #include \n> >>> @@ -443,16 +444,28 @@ static ssize_t ahci_show_em_supported(struct device *dev,\n> >>> void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> >>> {\n> >>> \tvoid __iomem *mmio = hpriv->mmio;\n> >>> -\tu32 cap, cap2, vers, port_map;\n> >>> +\tvoid __iomem *port_mmio;\n> >>> +\tunsigned long port_map;\n> >>> +\tu32 cap, cap2, vers;\n> >>> \tint i;\n> >>> \n> >>> \t/* make sure AHCI mode is enabled before accessing CAP */\n> >>> \tahci_enable_ahci(mmio);\n> >>> \n> >>> -\t/* Values prefixed with saved_ are written back to host after\n> >>> -\t * reset. Values without are used for driver operation.\n> >>> +\t/*\n> >>> +\t * Values prefixed with saved_ are written back to the HBA and ports\n> >>> +\t * registers after reset. Values without are used for driver operation.\n> >>> +\t */\n> >>> +\n> >>> +\t/*\n> >>> +\t * Override HW-init HBA capability fields with the platform-specific\n> >>> +\t * values. The rest of the HBA capabilities are defined as Read-only\n> >>> +\t * and can't be modified in CSR anyway.\n> >>> \t */\n> >>> -\thpriv->saved_cap = cap = readl(mmio + HOST_CAP);\n> >>> +\tcap = readl(mmio + HOST_CAP);\n> >>> +\tif (hpriv->saved_cap)\n> >>> +\t\tcap = (cap & ~(HOST_CAP_SSS | HOST_CAP_MPS)) | hpriv->saved_cap;\n> >>> +\thpriv->saved_cap = cap;\n> >>> \n> >>> \t/* CAP2 register is only defined for AHCI 1.2 and later */\n> >>> \tvers = readl(mmio + HOST_VERSION);\n> >>> @@ -519,7 +532,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> >>> \t/* Override the HBA ports mapping if the platform needs it */\n> >>> \tport_map = readl(mmio + HOST_PORTS_IMPL);\n> >>> \tif (hpriv->saved_port_map && port_map != hpriv->saved_port_map) {\n> >>> -\t\tdev_info(dev, \"forcing port_map 0x%x -> 0x%x\\n\",\n> >>> +\t\tdev_info(dev, \"forcing port_map 0x%lx -> 0x%x\\n\",\n> >>\n> > \n> >> This change is not necessary.\n> > \n> > It is. The port_map type has been changed.\n> \n> Ignore. When I read the patches the other day, the mailer font had that \"l\" look\n> like a \"1\" :) My mistake.\n\nOk.)\n\n-Sergey\n\n> \n> > \n> >>\n> >>> \t\t\t port_map, hpriv->saved_port_map);\n> >>> \t\tport_map = hpriv->saved_port_map;\n> >>> \t} else {\n> >>> @@ -527,7 +540,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> >>> \t}\n> >>> \n> >>> \tif (hpriv->mask_port_map) {\n> >>> -\t\tdev_warn(dev, \"masking port_map 0x%x -> 0x%x\\n\",\n> >>> +\t\tdev_warn(dev, \"masking port_map 0x%lx -> 0x%lx\\n\",\n> >>\n> >> Same.\n> > \n> > ditto\n> > \n> >>\n> >>> \t\t\tport_map,\n> >>> \t\t\tport_map & hpriv->mask_port_map);\n> >>> \t\tport_map &= hpriv->mask_port_map;\n> >>> @@ -546,7 +559,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> >>> \t\t */\n> >>> \t\tif (map_ports > ahci_nr_ports(cap)) {\n> >>> \t\t\tdev_warn(dev,\n> >>> -\t\t\t\t \"implemented port map (0x%x) contains more ports than nr_ports (%u), using nr_ports\\n\",\n> >>> +\t\t\t\t \"implemented port map (0x%lx) contains more ports than nr_ports (%u), using nr_ports\\n\",\n> >>\n> >> Same.\n> > \n> > ditto.\n> > \n> >>\n> >>> \t\t\t\t port_map, ahci_nr_ports(cap));\n> >>> \t\t\tport_map = 0;\n> >>> \t\t}\n> >>> @@ -555,12 +568,26 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)\n> >>> \t/* fabricate port_map from cap.nr_ports for < AHCI 1.3 */\n> >>> \tif (!port_map && vers < 0x10300) {\n> >>> \t\tport_map = (1 << ahci_nr_ports(cap)) - 1;\n> >>> -\t\tdev_warn(dev, \"forcing PORTS_IMPL to 0x%x\\n\", port_map);\n> >>> +\t\tdev_warn(dev, \"forcing PORTS_IMPL to 0x%lx\\n\", port_map);\n> >>\n> >> And again not needed.\n> > \n> > and ditto.\n> > \n> >>\n> >>> \n> >>> \t\t/* write the fixed up value to the PI register */\n> >>> \t\thpriv->saved_port_map = port_map;\n> >>> \t}\n> >>> \n> >>> +\t/*\n> >>> +\t * Preserve the ports capabilities defined by the platform. Note there\n> >>> +\t * is no need in storing the rest of the P#.CMD fields since they are\n> >>> +\t * volatile.\n> >>> +\t */\n> >>> +\tfor_each_set_bit(i, &port_map, AHCI_MAX_PORTS) {\n> >>> +\t\tif (hpriv->saved_port_cap[i])\n> >>> +\t\t\tcontinue;\n> >>> +\n> >>> +\t\tport_mmio = __ahci_port_base(hpriv, i);\n> >>> +\t\thpriv->saved_port_cap[i] =\n> >>> +\t\t\treadl(port_mmio + PORT_CMD) & PORT_CMD_CAP;\n> >>> +\t}\n> >>> +\n> >>> \t/* record values to use during operation */\n> >>> \thpriv->cap = cap;\n> >>> \thpriv->cap2 = cap2;\n> >>> @@ -590,13 +617,21 @@ EXPORT_SYMBOL_GPL(ahci_save_initial_config);\n> >>> static void ahci_restore_initial_config(struct ata_host *host)\n> >>> {\n> >>> \tstruct ahci_host_priv *hpriv = host->private_data;\n> >>> +\tunsigned long port_map = hpriv->port_map;\n> >>> \tvoid __iomem *mmio = hpriv->mmio;\n> >>> +\tvoid __iomem *port_mmio;\n> >>> +\tint i;\n> >>> \n> >>> \twritel(hpriv->saved_cap, mmio + HOST_CAP);\n> >>> \tif (hpriv->saved_cap2)\n> >>> \t\twritel(hpriv->saved_cap2, mmio + HOST_CAP2);\n> >>> \twritel(hpriv->saved_port_map, mmio + HOST_PORTS_IMPL);\n> >>> \t(void) readl(mmio + HOST_PORTS_IMPL);\t/* flush */\n> >>> +\n> >>> +\tfor_each_set_bit(i, &port_map, AHCI_MAX_PORTS) {\n> >>> +\t\tport_mmio = __ahci_port_base(hpriv, i);\n> >>> +\t\twritel(hpriv->saved_port_cap[i], port_mmio + PORT_CMD);\n> >>> +\t}\n> >>> }\n> >>> \n> >>> static unsigned ahci_scr_offset(struct ata_port *ap, unsigned int sc_reg)\n> >>> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c\n> >>> index efe640603f3f..8b542a8bc487 100644\n> >>> --- a/drivers/ata/libahci_platform.c\n> >>> +++ b/drivers/ata/libahci_platform.c\n> >>> @@ -23,6 +23,7 @@\n> >>> #include \n> >>> #include \n> >>> #include \n> >>> +\n> >>\n> >> white line change.\n> > \n> > Ok. I'll drop it.\n> > \n> > -Sergey\n> > \n> >>\n> >>> #include \"ahci.h\"\n> >>> \n> >>> static void ahci_host_stop(struct ata_host *host);\n> >>> @@ -383,6 +384,34 @@ static int ahci_platform_get_regulator(struct ahci_host_priv *hpriv, u32 port,\n> >>> \treturn rc;\n> >>> }\n> >>> \n> >>> +static int ahci_platform_get_firmware(struct ahci_host_priv *hpriv,\n> >>> +\t\t\t\t struct device *dev)\n> >>> +{\n> >>> +\tstruct device_node *child;\n> >>> +\tu32 port;\n> >>> +\n> >>> +\tif (!of_property_read_u32(dev->of_node, \"hba-cap\", &hpriv->saved_cap))\n> >>> +\t\thpriv->saved_cap &= (HOST_CAP_SSS | HOST_CAP_MPS);\n> >>> +\n> >>> +\tof_property_read_u32(dev->of_node,\n> >>> +\t\t\t \"ports-implemented\", &hpriv->saved_port_map);\n> >>> +\n> >>> +\tfor_each_child_of_node(dev->of_node, child) {\n> >>> +\t\tif (!of_device_is_available(child))\n> >>> +\t\t\tcontinue;\n> >>> +\n> >>> +\t\tif (of_property_read_u32(child, \"reg\", &port)) {\n> >>> +\t\t\tof_node_put(child);\n> >>> +\t\t\treturn -EINVAL;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\tif (!of_property_read_u32(child, \"hba-port-cap\", &hpriv->saved_port_cap[port]))\n> >>> +\t\t\thpriv->saved_port_cap[port] &= PORT_CMD_CAP;\n> >>> +\t}\n> >>> +\n> >>> +\treturn 0;\n> >>> +}\n> >>> +\n> >>> /**\n> >>> * ahci_platform_get_resources - Get platform resources\n> >>> * @pdev: platform device to get resources for\n> >>> @@ -523,9 +552,6 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,\n> >>> \t\tgoto err_out;\n> >>> \t}\n> >>> \n> >>> -\tof_property_read_u32(dev->of_node,\n> >>> -\t\t\t \"ports-implemented\", &hpriv->saved_port_map);\n> >>> -\n> >>> \tif (child_nodes) {\n> >>> \t\tfor_each_child_of_node(dev->of_node, child) {\n> >>> \t\t\tu32 port;\n> >>> @@ -590,6 +616,15 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,\n> >>> \t\tif (rc == -EPROBE_DEFER)\n> >>> \t\t\tgoto err_out;\n> >>> \t}\n> >>> +\n> >>> +\t/*\n> >>> +\t * Retrieve firmware-specific flags which then will be used to set\n> >>> +\t * the HW-init fields of HBA and its ports\n> >>> +\t */\n> >>> +\trc = ahci_platform_get_firmware(hpriv, dev);\n> >>> +\tif (rc)\n> >>> +\t\tgoto err_out;\n> >>> +\n> >>> \tpm_runtime_enable(dev);\n> >>> \tpm_runtime_get_sync(dev);\n> >>> \thpriv->got_runtime_pm = true;\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=iZtZd6M+;\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 4LPrMQ0lBlz9sG0\n\tfor ; Sat, 18 Jun 2022 06:32:50 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S1346994AbiFQUcs (ORCPT );\n Fri, 17 Jun 2022 16:32:48 -0400","from lindbergh.monkeyblade.net ([23.128.96.19]:33876 \"EHLO\n lindbergh.monkeyblade.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S229952AbiFQUcs (ORCPT\n ); Fri, 17 Jun 2022 16:32:48 -0400","from mail-lf1-x12f.google.com (mail-lf1-x12f.google.com\n [IPv6:2a00:1450:4864:20::12f])\n by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6C4599FFD;\n Fri, 17 Jun 2022 13:32:46 -0700 (PDT)","by mail-lf1-x12f.google.com with SMTP id a2so8538806lfg.5;\n Fri, 17 Jun 2022 13:32:46 -0700 (PDT)","from mobilestation ([95.79.189.214])\n by smtp.gmail.com with ESMTPSA id\n j10-20020a056512344a00b0047939239567sm752948lfr.240.2022.06.17.13.32.43\n (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n Fri, 17 Jun 2022 13:32: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=Nt7BK1f6cRFHz6pLGJSxHQltGnG9CIf3ikjX+Fr4Z0c=;\n b=iZtZd6M+uTIfLBlp270F0Meh1CS3eDV5RHh0StqLHjHyXEXHs5Nppx1XK5u7JQFthi\n 6bh8XC05k/qvhk0wG/MNjGl26sOHWSZkJT9r3viYgvtqFKocHdHcXjJs9YxmmEDKLk4N\n ckgItRtZ535KiqlH8Y6bqGMnl+H7fuUmybY/tSDexrXzIfK/gXy4EACP0okZ8LvyhqPL\n x2ejyKULTvtaEhQaVdw5eB8D+x6W67SKT4KOkQ7vgVywnkMVIQSF+27bHGi6Huah2KAk\n WfBK1m2EHbz576XCtxNaK0eyB1pB9OC5BSbPchGneuvh37HOPgnA19qJo50q6b0KHC0F\n w+ZA==","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=Nt7BK1f6cRFHz6pLGJSxHQltGnG9CIf3ikjX+Fr4Z0c=;\n b=djYBwgIkRSXVB7Zx4ctToHH1hjhu1BODOVfDVw8iz8bqqvnnEAzHseevFsuhgQgB0y\n s6hD0AhuFkjpKCc4NiqVY5VjiNHyc+LujxZw1OvvxI+UY8kng9VxRonCxXWagwDaEvzj\n xFq/dnji3UtXZqEk/lIW/GbLA3ecgxaBi6GYlk/4JiOgP2sTNrsYI5IClg0FLT+gF4XF\n sHl1/kkGSC+vBBhH7CDt5vrAvmLtI74wpKGnn26cIX5EAHDliOVaI0fmmf7UDos6AKKe\n j3ZjqLxxN0kJlO9nxPsy9450HQDHXQOaqFZhKn4W6c+5h4m9Op995ckME79HTp49Zjcp\n aAGQ==","X-Gm-Message-State":"AJIora+xRLvK1LG7/c2u06d2/4Dbr5PYbMgj3RhDa3WXuJ74fgH46Zp4\n oyJJp24iwBB3zSat131/9/U=","X-Google-Smtp-Source":"\n AGRyM1vW2061WCMUWRb0OHVW1/LX+0Q11W4LRpcWpCFqKpxFHErX9s6W+TmseVwJHw2QzXUPIT2xog==","X-Received":"by 2002:a05:6512:2623:b0:47d:ace7:c804 with SMTP id\n bt35-20020a056512262300b0047dace7c804mr6470255lfb.647.1655497964702;\n Fri, 17 Jun 2022 13:32:44 -0700 (PDT)","Date":"Fri, 17 Jun 2022 23:32: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 16/23] ata: ahci: Introduce firmware-specific caps\n initialization","Message-ID":"<20220617203242.3ujyknllrx2frzmq@mobilestation>","References":"<20220610081801.11854-1-Sergey.Semin@baikalelectronics.ru>\n <20220610081801.11854-17-Sergey.Semin@baikalelectronics.ru>\n \n <20220615211134.2wxzizbpmfl2akjh@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"}}]