From e8866e26f5e8ec551eec73dca1c30d458f9dca86 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Fri, 11 Apr 2025 07:59:09 +0200 Subject: [PATCH 01/18] ata: libata-core: Simplify ata_print_version_once Use dev_dbg_once() instead of open-coding the once functionality. Signed-off-by: Heiner Kallweit Signed-off-by: Damien Le Moal --- drivers/ata/libata-core.c | 6 ------ include/linux/libata.h | 17 +++++------------ 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 773799cfd443..79b20da0a256 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -6682,12 +6682,6 @@ const struct ata_port_info ata_dummy_port_info = { }; EXPORT_SYMBOL_GPL(ata_dummy_port_info); -void ata_print_version(const struct device *dev, const char *version) -{ - dev_printk(KERN_DEBUG, dev, "version %s\n", version); -} -EXPORT_SYMBOL(ata_print_version); - EXPORT_TRACEPOINT_SYMBOL_GPL(ata_tf_load); EXPORT_TRACEPOINT_SYMBOL_GPL(ata_exec_command); EXPORT_TRACEPOINT_SYMBOL_GPL(ata_bmdma_setup); diff --git a/include/linux/libata.h b/include/linux/libata.h index e5695998acb0..98affa1d9136 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -41,17 +41,6 @@ */ #undef ATA_IRQ_TRAP /* define to ack screaming irqs */ - -#define ata_print_version_once(dev, version) \ -({ \ - static bool __print_once; \ - \ - if (!__print_once) { \ - __print_once = true; \ - ata_print_version(dev, version); \ - } \ -}) - /* defines only for the constants which don't work well as enums */ #define ATA_TAG_POISON 0xfafbfcfdU @@ -1593,7 +1582,11 @@ do { \ #define ata_dev_dbg(dev, fmt, ...) \ ata_dev_printk(debug, dev, fmt, ##__VA_ARGS__) -void ata_print_version(const struct device *dev, const char *version); +static inline void ata_print_version_once(const struct device *dev, + const char *version) +{ + dev_dbg_once(dev, "version %s\n", version); +} /* * ata_eh_info helpers From f54464458d34141911047258090f25c67204cda4 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Wed, 16 Apr 2025 11:31:30 +0200 Subject: [PATCH 02/18] ata: libata-sata: Simplify sense_valid fetching While the SENSE DATA VALID field in the ACS-6 specification is 47 bits, we are currently only fetching 32 bits, because these are the only bits that we care about (these bits represent the tags (which can be 0-31)). Thus, replace the existing logic with a simple get_unaligned_le32(). While at it, change the type of sense_valid to u32. Signed-off-by: Niklas Cassel Reviewed-by: Hannes Reinecke Reviewed-by: Igor Pylypiv Signed-off-by: Damien Le Moal --- drivers/ata/libata-sata.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c index 2e4463d3a356..89d3b784706b 100644 --- a/drivers/ata/libata-sata.c +++ b/drivers/ata/libata-sata.c @@ -1509,9 +1509,10 @@ int ata_eh_get_ncq_success_sense(struct ata_link *link) struct ata_queued_cmd *qc; unsigned int err_mask, tag; u8 *sense, sk = 0, asc = 0, ascq = 0; - u64 sense_valid, val; u16 extended_sense; bool aux_icc_valid; + u32 sense_valid; + u64 val; int ret = 0; err_mask = ata_read_log_page(dev, ATA_LOG_SENSE_NCQ, 0, buf, 2); @@ -1529,8 +1530,7 @@ int ata_eh_get_ncq_success_sense(struct ata_link *link) return -EIO; } - sense_valid = (u64)buf[8] | ((u64)buf[9] << 8) | - ((u64)buf[10] << 16) | ((u64)buf[11] << 24); + sense_valid = get_unaligned_le32(&buf[8]); extended_sense = get_unaligned_le16(&buf[14]); aux_icc_valid = extended_sense & BIT(15); @@ -1545,7 +1545,7 @@ int ata_eh_get_ncq_success_sense(struct ata_link *link) * If the command does not have any sense data, clear ATA_SENSE. * Keep ATA_QCFLAG_EH_SUCCESS_CMD so that command is finished. */ - if (!(sense_valid & (1ULL << tag))) { + if (!(sense_valid & (1 << tag))) { qc->result_tf.status &= ~ATA_SENSE; continue; } From ecd9ecc75d150b82a832eb7aaa9b7b983fea5271 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Wed, 16 Apr 2025 11:31:31 +0200 Subject: [PATCH 03/18] ata: libata-sata: Use BIT() macro to convert tag to bit field The BIT() macro is commonly used in the kernel. Make use of it when converting a tag, fetched from the Successful NCQ Commands log or the NCQ Command Error log, to a bit field. This makes the code easier to read. Suggested-by: Igor Pylypiv Signed-off-by: Niklas Cassel Reviewed-by: Hannes Reinecke Reviewed-by: Igor Pylypiv Signed-off-by: Damien Le Moal --- drivers/ata/libata-sata.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c index 89d3b784706b..4e3034af285e 100644 --- a/drivers/ata/libata-sata.c +++ b/drivers/ata/libata-sata.c @@ -1545,7 +1545,7 @@ int ata_eh_get_ncq_success_sense(struct ata_link *link) * If the command does not have any sense data, clear ATA_SENSE. * Keep ATA_QCFLAG_EH_SUCCESS_CMD so that command is finished. */ - if (!(sense_valid & (1 << tag))) { + if (!(sense_valid & BIT(tag))) { qc->result_tf.status &= ~ATA_SENSE; continue; } @@ -1634,7 +1634,7 @@ void ata_eh_analyze_ncq_error(struct ata_link *link) return; } - if (!(link->sactive & (1 << tag))) { + if (!(link->sactive & BIT(tag))) { ata_link_err(link, "log page 10h reported inactive tag %d\n", tag); return; From 23a8e0df49b851ed1ad12f87c52d113be8a6b6e2 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Tue, 22 Apr 2025 10:09:40 +0100 Subject: [PATCH 04/18] ata: sata_sx4: Fix spelling mistake "parttern" -> "pattern" There are spelling mistakes in arrays test_parttern1 and test_parttern2. Fix them. Signed-off-by: Colin Ian King Signed-off-by: Damien Le Moal --- drivers/ata/sata_sx4.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c index c3042eca6332..f7f5131af937 100644 --- a/drivers/ata/sata_sx4.c +++ b/drivers/ata/sata_sx4.c @@ -1301,32 +1301,32 @@ static unsigned int pdc20621_dimm_init(struct ata_host *host) } if (dimm_test) { - u8 test_parttern1[40] = + u8 test_pattern1[40] = {0x55,0xAA,'P','r','o','m','i','s','e',' ', 'N','o','t',' ','Y','e','t',' ', 'D','e','f','i','n','e','d',' ', '1','.','1','0', '9','8','0','3','1','6','1','2',0,0}; - u8 test_parttern2[40] = {0}; + u8 test_pattern2[40] = {0}; - pdc20621_put_to_dimm(host, test_parttern2, 0x10040, 40); - pdc20621_put_to_dimm(host, test_parttern2, 0x40, 40); + pdc20621_put_to_dimm(host, test_pattern2, 0x10040, 40); + pdc20621_put_to_dimm(host, test_pattern2, 0x40, 40); - pdc20621_put_to_dimm(host, test_parttern1, 0x10040, 40); - pdc20621_get_from_dimm(host, test_parttern2, 0x40, 40); - dev_info(host->dev, "DIMM test pattern 1: %x, %x, %s\n", test_parttern2[0], - test_parttern2[1], &(test_parttern2[2])); - pdc20621_get_from_dimm(host, test_parttern2, 0x10040, + pdc20621_put_to_dimm(host, test_pattern1, 0x10040, 40); + pdc20621_get_from_dimm(host, test_pattern2, 0x40, 40); + dev_info(host->dev, "DIMM test pattern 1: %x, %x, %s\n", test_pattern2[0], + test_pattern2[1], &(test_pattern2[2])); + pdc20621_get_from_dimm(host, test_pattern2, 0x10040, 40); dev_info(host->dev, "DIMM test pattern 2: %x, %x, %s\n", - test_parttern2[0], - test_parttern2[1], &(test_parttern2[2])); + test_pattern2[0], + test_pattern2[1], &(test_pattern2[2])); - pdc20621_put_to_dimm(host, test_parttern1, 0x40, 40); - pdc20621_get_from_dimm(host, test_parttern2, 0x40, 40); + pdc20621_put_to_dimm(host, test_pattern1, 0x40, 40); + pdc20621_get_from_dimm(host, test_pattern2, 0x40, 40); dev_info(host->dev, "DIMM test pattern 3: %x, %x, %s\n", - test_parttern2[0], - test_parttern2[1], &(test_parttern2[2])); + test_pattern2[0], + test_pattern2[1], &(test_pattern2[2])); } /* ECC initiliazation. */ From 11533932f5c506f66281a147ff8469b97c108ab4 Mon Sep 17 00:00:00 2001 From: Igor Pylypiv Date: Tue, 22 Apr 2025 10:21:23 -0700 Subject: [PATCH 05/18] ata: libata-scsi: Do not set the INFORMATION field twice for ATA PT For ATA PASS-THROUGH + fixed format sense data + NCQ autosense the INFORMATION sense data field is being written twice: - 1st write: (redundant) scsi_set_sense_information() sets the INFORMATION field to ATA LBA. This is incorrect for ATA PASS-THROUGH. - 2nd write: (correct) ata_scsi_set_passthru_sense_fields() sets the INFORMATION field to ATA ERROR/STATUS/DEVICE/COUNT(7:0) as per SAT spec. There is no user-visible issue because second write overwrites the incorrect data from the first write. This patch eliminates the reduntant write by moving the INFORMATION sense data field population logic to ata_scsi_qc_complete(). Signed-off-by: Igor Pylypiv Reviewed-by: Niklas Cassel Signed-off-by: Damien Le Moal --- drivers/ata/libata-sata.c | 2 -- drivers/ata/libata-scsi.c | 31 ++++++++++++++----------------- drivers/ata/libata.h | 3 --- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c index 4e3034af285e..cb46ce276bb1 100644 --- a/drivers/ata/libata-sata.c +++ b/drivers/ata/libata-sata.c @@ -1659,8 +1659,6 @@ void ata_eh_analyze_ncq_error(struct ata_link *link) if (ata_scsi_sense_is_valid(sense_key, asc, ascq)) { ata_scsi_set_sense(dev, qc->scsicmd, sense_key, asc, ascq); - ata_scsi_set_sense_information(dev, qc->scsicmd, - &qc->result_tf); qc->flags |= ATA_QCFLAG_SENSE_VALID; } } diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 2796c0da8257..ef117a0bc248 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -216,17 +216,21 @@ void ata_scsi_set_sense(struct ata_device *dev, struct scsi_cmnd *cmd, scsi_build_sense(cmd, d_sense, sk, asc, ascq); } -void ata_scsi_set_sense_information(struct ata_device *dev, - struct scsi_cmnd *cmd, - const struct ata_taskfile *tf) +static void ata_scsi_set_sense_information(struct ata_queued_cmd *qc) { u64 information; - information = ata_tf_read_block(tf, dev); + if (!(qc->flags & ATA_QCFLAG_RTF_FILLED)) { + ata_dev_dbg(qc->dev, + "missing result TF: can't set INFORMATION sense field\n"); + return; + } + + information = ata_tf_read_block(&qc->result_tf, qc->dev); if (information == U64_MAX) return; - scsi_set_sense_information(cmd->sense_buffer, + scsi_set_sense_information(qc->scsicmd->sense_buffer, SCSI_SENSE_BUFFERSIZE, information); } @@ -971,8 +975,7 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc) * ata_gen_ata_sense - generate a SCSI fixed sense block * @qc: Command that we are erroring out * - * Generate sense block for a failed ATA command @qc. Descriptor - * format is used to accommodate LBA48 block address. + * Generate sense block for a failed ATA command @qc. * * LOCKING: * None. @@ -982,8 +985,6 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc) struct ata_device *dev = qc->dev; struct scsi_cmnd *cmd = qc->scsicmd; struct ata_taskfile *tf = &qc->result_tf; - unsigned char *sb = cmd->sense_buffer; - u64 block; u8 sense_key, asc, ascq; if (ata_dev_disabled(dev)) { @@ -1014,12 +1015,6 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc) ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0, 0); return; } - - block = ata_tf_read_block(&qc->result_tf, dev); - if (block == U64_MAX) - return; - - scsi_set_sense_information(sb, SCSI_SENSE_BUFFERSIZE, block); } void ata_scsi_sdev_config(struct scsi_device *sdev) @@ -1679,8 +1674,10 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc) ata_scsi_set_passthru_sense_fields(qc); if (is_ck_cond_request) set_status_byte(qc->scsicmd, SAM_STAT_CHECK_CONDITION); - } else if (is_error && !have_sense) { - ata_gen_ata_sense(qc); + } else if (is_error) { + if (!have_sense) + ata_gen_ata_sense(qc); + ata_scsi_set_sense_information(qc); } ata_qc_done(qc); diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h index 0337be4faec7..ce5c628fa6fd 100644 --- a/drivers/ata/libata.h +++ b/drivers/ata/libata.h @@ -141,9 +141,6 @@ extern int ata_scsi_offline_dev(struct ata_device *dev); extern bool ata_scsi_sense_is_valid(u8 sk, u8 asc, u8 ascq); extern void ata_scsi_set_sense(struct ata_device *dev, struct scsi_cmnd *cmd, u8 sk, u8 asc, u8 ascq); -extern void ata_scsi_set_sense_information(struct ata_device *dev, - struct scsi_cmnd *cmd, - const struct ata_taskfile *tf); extern void ata_scsi_media_change_notify(struct ata_device *dev); extern void ata_scsi_hotplug(struct work_struct *work); extern void ata_scsi_dev_rescan(struct work_struct *work); From b8ed9475384fd78e7650b7c4a403c1958a765a74 Mon Sep 17 00:00:00 2001 From: Nicolas Frattaroli Date: Thu, 24 Apr 2025 20:52:22 +0200 Subject: [PATCH 06/18] dt-bindings: ata: rockchip-dwc-ahci: add RK3576 compatible The Rockchip RK3576 has two SATA controllers. They work the same as the RK3568 SATA controllers, having the same number of clocks and ports. Signed-off-by: Nicolas Frattaroli Acked-by: Krzysztof Kozlowski Reviewed-by: Niklas Cassel Link: https://lore.kernel.org/r/20250424-rk3576-sata-v1-1-23ee89c939fe@collabora.com Signed-off-by: Niklas Cassel --- Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml b/Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml index 13eaa8d9a16e..b5ecaabfe2e2 100644 --- a/Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml +++ b/Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml @@ -20,6 +20,7 @@ select: contains: enum: - rockchip,rk3568-dwc-ahci + - rockchip,rk3576-dwc-ahci - rockchip,rk3588-dwc-ahci required: - compatible @@ -29,6 +30,7 @@ properties: items: - enum: - rockchip,rk3568-dwc-ahci + - rockchip,rk3576-dwc-ahci - rockchip,rk3588-dwc-ahci - const: snps,dwc-ahci @@ -83,6 +85,7 @@ allOf: contains: enum: - rockchip,rk3568-dwc-ahci + - rockchip,rk3576-dwc-ahci then: properties: clocks: From 439d47608bb3e5f7b5f5eb55c568576b60731c4d Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Mon, 12 May 2025 10:44:00 +0200 Subject: [PATCH 07/18] ata: libata: Print if port is external on boot Commit affccb16c117 ("ata: ahci: print the lpm policy on boot") added a lpm-pol print during boot, which shows the LPM policy used by each port. While the LPM policy is usually determined by the Kconfig CONFIG_SATA_MOBILE_LPM_POLICY, the Kconfig value is overridden e.g. if firmware has marked the port as hotplug capable / external. Commit f97106b10d9a ("ata: ahci: Add debug print for external port") did add a debug print to show if LPM was disabled because firmware has marked the port as external, however, because devices having broken LPM (even though they claim to support it) is more common than one would have hoped, print "ext" during boot if firmware has marked the port is external. This will make it easier to debug certain LPM issues, e.g. if firmware has enabled/marked only some of the ports as hotplug capable / external. Before (port marked as external by firmware): ata1: SATA max UDMA/133 abar m4096@0xfebd3000 port 0xfebd3100 irq 57 lpm-pol 0 After (port marked as external by firmware): ata1: SATA max UDMA/133 abar m4096@0xfebd3000 port 0xfebd3100 irq 57 lpm-pol 0 ext Signed-off-by: Niklas Cassel Signed-off-by: Damien Le Moal --- include/linux/libata.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/linux/libata.h b/include/linux/libata.h index 98affa1d9136..31be45fd47a6 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1618,6 +1618,8 @@ static inline void ata_port_desc_misc(struct ata_port *ap, int irq) { ata_port_desc(ap, "irq %d", irq); ata_port_desc(ap, "lpm-pol %d", ap->target_lpm_policy); + if (ap->pflags & ATA_PFLAG_EXTERNAL) + ata_port_desc(ap, "ext"); } static inline bool ata_tag_internal(unsigned int tag) From f07f2b3fecac4096b2c89f2b89b5f564c279cfc8 Mon Sep 17 00:00:00 2001 From: "Rob Herring (Arm)" Date: Mon, 12 May 2025 16:57:05 -0500 Subject: [PATCH 08/18] dt-bindings: ata: Convert ti,dm816-ahci to DT schema Convert the TI DM816 AHCI SATA Controller to DT schema format. It's a straight-forward conversion. Signed-off-by: Rob Herring (Arm) Reviewed-by: Bartosz Golaszewski Signed-off-by: Damien Le Moal --- .../devicetree/bindings/ata/ahci-dm816.txt | 21 --------- .../bindings/ata/ti,dm816-ahci.yaml | 43 +++++++++++++++++++ 2 files changed, 43 insertions(+), 21 deletions(-) delete mode 100644 Documentation/devicetree/bindings/ata/ahci-dm816.txt create mode 100644 Documentation/devicetree/bindings/ata/ti,dm816-ahci.yaml diff --git a/Documentation/devicetree/bindings/ata/ahci-dm816.txt b/Documentation/devicetree/bindings/ata/ahci-dm816.txt deleted file mode 100644 index f8c535f3541f..000000000000 --- a/Documentation/devicetree/bindings/ata/ahci-dm816.txt +++ /dev/null @@ -1,21 +0,0 @@ -Device tree binding for the TI DM816 AHCI SATA Controller ---------------------------------------------------------- - -Required properties: - - compatible: must be "ti,dm816-ahci" - - reg: physical base address and size of the register region used by - the controller (as defined by the AHCI 1.1 standard) - - interrupts: interrupt specifier (refer to the interrupt binding) - - clocks: list of phandle and clock specifier pairs (or only - phandles for clock providers with '0' defined for - #clock-cells); two clocks must be specified: the functional - clock and an external reference clock - -Example: - - sata: sata@4a140000 { - compatible = "ti,dm816-ahci"; - reg = <0x4a140000 0x10000>; - interrupts = <16>; - clocks = <&sysclk5_ck>, <&sata_refclk>; - }; diff --git a/Documentation/devicetree/bindings/ata/ti,dm816-ahci.yaml b/Documentation/devicetree/bindings/ata/ti,dm816-ahci.yaml new file mode 100644 index 000000000000..d0ff9e78afe6 --- /dev/null +++ b/Documentation/devicetree/bindings/ata/ti,dm816-ahci.yaml @@ -0,0 +1,43 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/ata/ti,dm816-ahci.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TI DM816 AHCI SATA Controller + +maintainers: + - Bartosz Golaszewski + +allOf: + - $ref: ahci-common.yaml# + +properties: + compatible: + const: ti,dm816-ahci + + reg: + maxItems: 1 + + clocks: + items: + - description: functional clock + - description: external reference clock + + ti,hwmods: + const: sata + +required: + - compatible + - clocks + +unevaluatedProperties: false + +examples: + - | + sata@4a140000 { + compatible = "ti,dm816-ahci"; + reg = <0x4a140000 0x10000>; + interrupts = <16>; + clocks = <&sysclk5_ck>, <&sata_refclk>; + }; From 2de72bb42c149ba7908291bd3dd46e315a7fe65f Mon Sep 17 00:00:00 2001 From: "Rob Herring (Arm)" Date: Mon, 12 May 2025 16:57:23 -0500 Subject: [PATCH 09/18] dt-bindings: ata: Convert st,ahci to DT schema Convert the ST AHCI SATA Controller to DT schema format. The phy-names changes from "ahci_phy" to "sata-phy" with the inclusion of ahci-common.yaml. That's an ABI change, but the Linux driver at least ignores the names. The binding uses "ports-implemented" property, so including ahci-common.yaml is required. Signed-off-by: Rob Herring (Arm) Signed-off-by: Damien Le Moal --- .../devicetree/bindings/ata/ahci-st.txt | 35 --------- .../devicetree/bindings/ata/st,ahci.yaml | 72 +++++++++++++++++++ 2 files changed, 72 insertions(+), 35 deletions(-) delete mode 100644 Documentation/devicetree/bindings/ata/ahci-st.txt create mode 100644 Documentation/devicetree/bindings/ata/st,ahci.yaml diff --git a/Documentation/devicetree/bindings/ata/ahci-st.txt b/Documentation/devicetree/bindings/ata/ahci-st.txt deleted file mode 100644 index 909c9935360d..000000000000 --- a/Documentation/devicetree/bindings/ata/ahci-st.txt +++ /dev/null @@ -1,35 +0,0 @@ -STMicroelectronics STi SATA controller - -This binding describes a SATA device. - -Required properties: - - compatible : Must be "st,ahci" - - reg : Physical base addresses and length of register sets - - interrupts : Interrupt associated with the SATA device - - interrupt-names : Associated name must be; "hostc" - - clocks : The phandle for the clock - - clock-names : Associated name must be; "ahci_clk" - - phys : The phandle for the PHY port - - phy-names : Associated name must be; "ahci_phy" - -Optional properties: - - resets : The power-down, soft-reset and power-reset lines of SATA IP - - reset-names : Associated names must be; "pwr-dwn", "sw-rst" and "pwr-rst" - -Example: - - /* Example for stih407 family silicon */ - sata0: sata@9b20000 { - compatible = "st,ahci"; - reg = <0x9b20000 0x1000>; - interrupts = ; - interrupt-names = "hostc"; - phys = <&phy_port0 PHY_TYPE_SATA>; - phy-names = "ahci_phy"; - resets = <&powerdown STIH407_SATA0_POWERDOWN>, - <&softreset STIH407_SATA0_SOFTRESET>, - <&softreset STIH407_SATA0_PWR_SOFTRESET>; - reset-names = "pwr-dwn", "sw-rst", "pwr-rst"; - clocks = <&clk_s_c0_flexgen CLK_ICN_REG>; - clock-names = "ahci_clk"; - }; diff --git a/Documentation/devicetree/bindings/ata/st,ahci.yaml b/Documentation/devicetree/bindings/ata/st,ahci.yaml new file mode 100644 index 000000000000..6e8e4b4f3d6c --- /dev/null +++ b/Documentation/devicetree/bindings/ata/st,ahci.yaml @@ -0,0 +1,72 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/ata/st,ahci.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: STMicroelectronics STi SATA controller + +maintainers: + - Patrice Chotard + +allOf: + - $ref: ahci-common.yaml# + +properties: + compatible: + const: st,ahci + + interrupt-names: + items: + - const: hostc + + clocks: + maxItems: 1 + + clock-names: + items: + - const: ahci_clk + + resets: + items: + - description: Power-down line + - description: Soft-reset line + - description: Power-reset line + + reset-names: + items: + - const: pwr-dwn + - const: sw-rst + - const: pwr-rst + +required: + - compatible + - interrupt-names + - phys + - phy-names + - clocks + - clock-names + +unevaluatedProperties: false + +examples: + - | + #include + #include + #include + #include + + sata@9b20000 { + compatible = "st,ahci"; + reg = <0x9b20000 0x1000>; + interrupts = ; + interrupt-names = "hostc"; + phys = <&phy_port0 PHY_TYPE_SATA>; + phy-names = "sata-phy"; + resets = <&powerdown STIH407_SATA0_POWERDOWN>, + <&softreset STIH407_SATA0_SOFTRESET>, + <&softreset STIH407_SATA0_PWR_SOFTRESET>; + reset-names = "pwr-dwn", "sw-rst", "pwr-rst"; + clocks = <&clk_s_c0_flexgen CLK_ICN_REG>; + clock-names = "ahci_clk"; + }; From b562d788e27ab0a8016951672793525e35bf6eec Mon Sep 17 00:00:00 2001 From: "Rob Herring (Arm)" Date: Mon, 12 May 2025 16:57:29 -0500 Subject: [PATCH 10/18] dt-bindings: ata: Convert apm,xgene-ahci to DT schema Convert the APM X-Gene AHCI SATA Controller to DT schema format. It's a straight-forward conversion. Signed-off-by: Rob Herring (Arm) Signed-off-by: Damien Le Moal --- .../bindings/ata/apm,xgene-ahci.yaml | 58 ++++++++++++++ .../devicetree/bindings/ata/apm-xgene.txt | 77 ------------------- 2 files changed, 58 insertions(+), 77 deletions(-) create mode 100644 Documentation/devicetree/bindings/ata/apm,xgene-ahci.yaml delete mode 100644 Documentation/devicetree/bindings/ata/apm-xgene.txt diff --git a/Documentation/devicetree/bindings/ata/apm,xgene-ahci.yaml b/Documentation/devicetree/bindings/ata/apm,xgene-ahci.yaml new file mode 100644 index 000000000000..7dc942808656 --- /dev/null +++ b/Documentation/devicetree/bindings/ata/apm,xgene-ahci.yaml @@ -0,0 +1,58 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/ata/apm,xgene-ahci.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: APM X-Gene 6.0 Gb/s SATA host controller + +maintainers: + - Rob Herring + +allOf: + - $ref: ahci-common.yaml# + +properties: + compatible: + enum: + - apm,xgene-ahci + - apm,xgene-ahci-pcie + + reg: + minItems: 4 + items: + - description: AHCI memory resource + - description: Host controller core + - description: Host controller diagnostic + - description: Host controller AXI + - description: Host controller MUX + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + +required: + - compatible + - clocks + - phys + - phy-names + +unevaluatedProperties: false + +examples: + - | + sata@1a400000 { + compatible = "apm,xgene-ahci"; + reg = <0x1a400000 0x1000>, + <0x1f220000 0x1000>, + <0x1f22d000 0x1000>, + <0x1f22e000 0x1000>, + <0x1f227000 0x1000>; + clocks = <&sataclk 0>; + dma-coherent; + interrupts = <0x0 0x87 0x4>; + phys = <&phy2 0>; + phy-names = "sata-phy"; + }; diff --git a/Documentation/devicetree/bindings/ata/apm-xgene.txt b/Documentation/devicetree/bindings/ata/apm-xgene.txt deleted file mode 100644 index 02e690a675db..000000000000 --- a/Documentation/devicetree/bindings/ata/apm-xgene.txt +++ /dev/null @@ -1,77 +0,0 @@ -* APM X-Gene 6.0 Gb/s SATA host controller nodes - -SATA host controller nodes are defined to describe on-chip Serial ATA -controllers. Each SATA controller (pair of ports) have its own node. - -Required properties: -- compatible : Shall contain: - * "apm,xgene-ahci" -- reg : First memory resource shall be the AHCI memory - resource. - Second memory resource shall be the host controller - core memory resource. - Third memory resource shall be the host controller - diagnostic memory resource. - 4th memory resource shall be the host controller - AXI memory resource. - 5th optional memory resource shall be the host - controller MUX memory resource if required. -- interrupts : Interrupt-specifier for SATA host controller IRQ. -- clocks : Reference to the clock entry. -- phys : A list of phandles + phy-specifiers, one for each - entry in phy-names. -- phy-names : Should contain: - * "sata-phy" for the SATA 6.0Gbps PHY - -Optional properties: -- dma-coherent : Present if dma operations are coherent -- status : Shall be "ok" if enabled or "disabled" if disabled. - Default is "ok". - -Example: - sataclk: sataclk { - compatible = "fixed-clock"; - #clock-cells = <1>; - clock-frequency = <100000000>; - clock-output-names = "sataclk"; - }; - - phy2: phy@1f22a000 { - compatible = "apm,xgene-phy"; - reg = <0x0 0x1f22a000 0x0 0x100>; - #phy-cells = <1>; - }; - - phy3: phy@1f23a000 { - compatible = "apm,xgene-phy"; - reg = <0x0 0x1f23a000 0x0 0x100>; - #phy-cells = <1>; - }; - - sata2: sata@1a400000 { - compatible = "apm,xgene-ahci"; - reg = <0x0 0x1a400000 0x0 0x1000>, - <0x0 0x1f220000 0x0 0x1000>, - <0x0 0x1f22d000 0x0 0x1000>, - <0x0 0x1f22e000 0x0 0x1000>, - <0x0 0x1f227000 0x0 0x1000>; - interrupts = <0x0 0x87 0x4>; - dma-coherent; - clocks = <&sataclk 0>; - phys = <&phy2 0>; - phy-names = "sata-phy"; - }; - - sata3: sata@1a800000 { - compatible = "apm,xgene-ahci-pcie"; - reg = <0x0 0x1a800000 0x0 0x1000>, - <0x0 0x1f230000 0x0 0x1000>, - <0x0 0x1f23d000 0x0 0x1000>, - <0x0 0x1f23e000 0x0 0x1000>, - <0x0 0x1f237000 0x0 0x1000>; - interrupts = <0x0 0x88 0x4>; - dma-coherent; - clocks = <&sataclk 0>; - phys = <&phy3 0>; - phy-names = "sata-phy"; - }; From ca7cf1f41f387ba5f015f96e17aa05dcd3cdc1dc Mon Sep 17 00:00:00 2001 From: "Rob Herring (Arm)" Date: Mon, 12 May 2025 16:57:41 -0500 Subject: [PATCH 11/18] dt-bindings: ata: Convert cavium,ebt3000-compact-flash to DT schema Convert the Cavium Compact Flash Controller to DT schema format. It's a straight-forward conversion. Signed-off-by: Rob Herring (Arm) Signed-off-by: Damien Le Moal --- .../ata/cavium,ebt3000-compact-flash.yaml | 59 +++++++++++++++++++ .../bindings/ata/cavium-compact-flash.txt | 30 ---------- 2 files changed, 59 insertions(+), 30 deletions(-) create mode 100644 Documentation/devicetree/bindings/ata/cavium,ebt3000-compact-flash.yaml delete mode 100644 Documentation/devicetree/bindings/ata/cavium-compact-flash.txt diff --git a/Documentation/devicetree/bindings/ata/cavium,ebt3000-compact-flash.yaml b/Documentation/devicetree/bindings/ata/cavium,ebt3000-compact-flash.yaml new file mode 100644 index 000000000000..349f289b81e6 --- /dev/null +++ b/Documentation/devicetree/bindings/ata/cavium,ebt3000-compact-flash.yaml @@ -0,0 +1,59 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/ata/cavium,ebt3000-compact-flash.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Cavium Compact Flash + +maintainers: + - Rob Herring + +description: + The Cavium Compact Flash device is connected to the Octeon Boot Bus, and is + thus a child of the Boot Bus device. It can read and write industry standard + compact flash devices. + +properties: + compatible: + const: cavium,ebt3000-compact-flash + + reg: + description: The base address of the CF chip select banks. + items: + - description: CF chip select bank 0 + - description: CF chip select bank 1 + + cavium,bus-width: + description: The width of the connection to the CF devices. + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [8, 16] + + cavium,true-ide: + description: True IDE mode when present. + type: boolean + + cavium,dma-engine-handle: + description: A phandle for the DMA Engine connected to this device. + $ref: /schemas/types.yaml#/definitions/phandle + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + bus { + #address-cells = <2>; + #size-cells = <1>; + + compact-flash@5,0 { + compatible = "cavium,ebt3000-compact-flash"; + reg = <5 0 0x10000>, <6 0 0x10000>; + cavium,bus-width = <16>; + cavium,true-ide; + cavium,dma-engine-handle = <&dma0>; + }; + }; diff --git a/Documentation/devicetree/bindings/ata/cavium-compact-flash.txt b/Documentation/devicetree/bindings/ata/cavium-compact-flash.txt deleted file mode 100644 index 3bacc8e0931e..000000000000 --- a/Documentation/devicetree/bindings/ata/cavium-compact-flash.txt +++ /dev/null @@ -1,30 +0,0 @@ -* Compact Flash - -The Cavium Compact Flash device is connected to the Octeon Boot Bus, -and is thus a child of the Boot Bus device. It can read and write -industry standard compact flash devices. - -Properties: -- compatible: "cavium,ebt3000-compact-flash"; - - Compatibility with many Cavium evaluation boards. - -- reg: The base address of the CF chip select banks. Depending on - the device configuration, there may be one or two banks. - -- cavium,bus-width: The width of the connection to the CF devices. Valid - values are 8 and 16. - -- cavium,true-ide: Optional, if present the CF connection is in True IDE mode. - -- cavium,dma-engine-handle: Optional, a phandle for the DMA Engine connected - to this device. - -Example: - compact-flash@5,0 { - compatible = "cavium,ebt3000-compact-flash"; - reg = <5 0 0x10000>, <6 0 0x10000>; - cavium,bus-width = <16>; - cavium,true-ide; - cavium,dma-engine-handle = <&dma0>; - }; From 6130ed3cd0b4c5e22757e7aa1819abd66f0987e3 Mon Sep 17 00:00:00 2001 From: "Rob Herring (Arm)" Date: Mon, 12 May 2025 16:57:48 -0500 Subject: [PATCH 12/18] dt-bindings: ata: Convert marvell,orion-sata to DT schema Convert the Marvell Orion SATA Controller to DT schema format. The clocks and clock-names properties were missing. The names for phy-names were incorrect. The maximum "nr-ports" was determined from the Linux driver. Signed-off-by: Rob Herring (Arm) Signed-off-by: Damien Le Moal --- .../bindings/ata/marvell,orion-sata.yaml | 83 +++++++++++++++++++ .../devicetree/bindings/ata/marvell.txt | 22 ----- 2 files changed, 83 insertions(+), 22 deletions(-) create mode 100644 Documentation/devicetree/bindings/ata/marvell,orion-sata.yaml delete mode 100644 Documentation/devicetree/bindings/ata/marvell.txt diff --git a/Documentation/devicetree/bindings/ata/marvell,orion-sata.yaml b/Documentation/devicetree/bindings/ata/marvell,orion-sata.yaml new file mode 100644 index 000000000000..f656ea9223d6 --- /dev/null +++ b/Documentation/devicetree/bindings/ata/marvell,orion-sata.yaml @@ -0,0 +1,83 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/ata/marvell,orion-sata.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Marvell Orion SATA + +maintainers: + - Andrew Lunn + - Gregory Clement + +allOf: + - $ref: sata-common.yaml# + +properties: + compatible: + enum: + - marvell,orion-sata + - marvell,armada-370-sata + + reg: + maxItems: 1 + + clocks: + minItems: 1 + maxItems: 8 + + clock-names: + minItems: 1 + items: + - const: '0' + - const: '1' + - const: '2' + - const: '3' + - const: '4' + - const: '5' + - const: '6' + - const: '7' + + interrupts: + maxItems: 1 + + nr-ports: + description: + Number of SATA ports in use. + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 8 + + phys: + minItems: 1 + maxItems: 8 + + phy-names: + minItems: 1 + items: + - const: port0 + - const: port1 + - const: port2 + - const: port3 + - const: port4 + - const: port5 + - const: port6 + - const: port7 + +required: + - compatible + - reg + - interrupts + - nr-ports + +unevaluatedProperties: false + +examples: + - | + sata@80000 { + compatible = "marvell,orion-sata"; + reg = <0x80000 0x5000>; + interrupts = <21>; + phys = <&sata_phy0>, <&sata_phy1>; + phy-names = "port0", "port1"; + nr-ports = <2>; + }; diff --git a/Documentation/devicetree/bindings/ata/marvell.txt b/Documentation/devicetree/bindings/ata/marvell.txt deleted file mode 100644 index b460edd12766..000000000000 --- a/Documentation/devicetree/bindings/ata/marvell.txt +++ /dev/null @@ -1,22 +0,0 @@ -* Marvell Orion SATA - -Required Properties: -- compatibility : "marvell,orion-sata" or "marvell,armada-370-sata" -- reg : Address range of controller -- interrupts : Interrupt controller is using -- nr-ports : Number of SATA ports in use. - -Optional Properties: -- phys : List of phandles to sata phys -- phy-names : Should be "0", "1", etc, one number per phandle - -Example: - - sata@80000 { - compatible = "marvell,orion-sata"; - reg = <0x80000 0x5000>; - interrupts = <21>; - phys = <&sata_phy0>, <&sata_phy1>; - phy-names = "0", "1"; - nr-ports = <2>; - } From 3b0bca979344b2e26de8f4c175bed975b1d54e8f Mon Sep 17 00:00:00 2001 From: "Rob Herring (Arm)" Date: Mon, 12 May 2025 16:57:56 -0500 Subject: [PATCH 13/18] dt-bindings: ata: Convert arasan,cf-spear1340 to DT schema Convert the Arasan/SPEAr Compact Flash Controller to DT schema format. The "clock-frequency" property isn't actually used. Add a single "clocks" entry as the Linux driver supports a single clock though the platform still doesn't have clocks in DT. Signed-off-by: Rob Herring (Arm) Signed-off-by: Damien Le Moal --- .../bindings/ata/arasan,cf-spear1340.yaml | 70 +++++++++++++++++++ .../devicetree/bindings/ata/pata-arasan.txt | 37 ---------- 2 files changed, 70 insertions(+), 37 deletions(-) create mode 100644 Documentation/devicetree/bindings/ata/arasan,cf-spear1340.yaml delete mode 100644 Documentation/devicetree/bindings/ata/pata-arasan.txt diff --git a/Documentation/devicetree/bindings/ata/arasan,cf-spear1340.yaml b/Documentation/devicetree/bindings/ata/arasan,cf-spear1340.yaml new file mode 100644 index 000000000000..4d7017452dda --- /dev/null +++ b/Documentation/devicetree/bindings/ata/arasan,cf-spear1340.yaml @@ -0,0 +1,70 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/ata/arasan,cf-spear1340.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Arasan PATA Compact Flash Controller + +maintainers: + - Viresh Kumar + +properties: + compatible: + const: arasan,cf-spear1340 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + + arasan,broken-udma: + description: UDMA mode is unusable + type: boolean + + arasan,broken-mwdma: + description: MWDMA mode is unusable + type: boolean + + arasan,broken-pio: + description: PIO mode is unusable + type: boolean + + dmas: + maxItems: 1 + + dma-names: + items: + - const: data + +required: + - compatible + - reg + - interrupts + +additionalProperties: false + +allOf: + - if: + not: + required: + - arasan,broken-udma + - arasan,broken-mwdma + then: + required: + - dmas + - dma-names + +examples: + - | + cf@fc000000 { + compatible = "arasan,cf-spear1340"; + reg = <0xfc000000 0x1000>; + interrupts = <12>; + dmas = <&dma 23>; + dma-names = "data"; + }; diff --git a/Documentation/devicetree/bindings/ata/pata-arasan.txt b/Documentation/devicetree/bindings/ata/pata-arasan.txt deleted file mode 100644 index 872edc105680..000000000000 --- a/Documentation/devicetree/bindings/ata/pata-arasan.txt +++ /dev/null @@ -1,37 +0,0 @@ -* ARASAN PATA COMPACT FLASH CONTROLLER - -Required properties: -- compatible: "arasan,cf-spear1340" -- reg: Address range of the CF registers -- interrupt: Should contain the CF interrupt number -- clock-frequency: Interface clock rate, in Hz, one of - 25000000 - 33000000 - 40000000 - 50000000 - 66000000 - 75000000 - 100000000 - 125000000 - 150000000 - 166000000 - 200000000 - -Optional properties: -- arasan,broken-udma: if present, UDMA mode is unusable -- arasan,broken-mwdma: if present, MWDMA mode is unusable -- arasan,broken-pio: if present, PIO mode is unusable -- dmas: one DMA channel, as described in bindings/dma/dma.txt - required unless both UDMA and MWDMA mode are broken -- dma-names: the corresponding channel name, must be "data" - -Example: - - cf@fc000000 { - compatible = "arasan,cf-spear1340"; - reg = <0xfc000000 0x1000>; - interrupt-parent = <&vic1>; - interrupts = <12>; - dmas = <&dma-controller 23>; - dma-names = "data"; - }; From 381d43b26282377a7e2f7ddfdd0147ad72353621 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 15 May 2025 15:56:23 +0200 Subject: [PATCH 14/18] ata: libata-eh: Update DIPM comments to reflect reality The comments describing which LPM policies that has DIPM enabled predates the introduction of the LPM policies ATA_LPM_MIN_POWER_WITH_PARTIAL and ATA_LPM_MED_POWER_WITH_DIPM. Update the DIPM comments to reflect reality. Also remove the sentence that claims that "Order device and link configurations such that the host always allows DIPM requests." This comment is written before 24e0e61db3cb ("ata: libata: disallow dev-initiated LPM transitions to unsupported states"). Even though the set_lpm() call is done before enabling DIPM, the host will not always allow DIPM requests. For all LPM polcies where DIPM is enabled, only DIPM requests to LPM states that are supported by the HBA will be allowed. Signed-off-by: Niklas Cassel Signed-off-by: Damien Le Moal --- drivers/ata/libata-eh.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index b990c1ee0b12..f39756a26751 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -3443,10 +3443,9 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy, return 0; /* - * DIPM is enabled only for MIN_POWER as some devices - * misbehave when the host NACKs transition to SLUMBER. Order - * device and link configurations such that the host always - * allows DIPM requests. + * DIPM is enabled only for ATA_LPM_MIN_POWER, + * ATA_LPM_MIN_POWER_WITH_PARTIAL, and ATA_LPM_MED_POWER_WITH_DIPM, as + * some devices misbehave when the host NACKs transition to SLUMBER. */ ata_for_each_dev(dev, link, ENABLED) { bool hipm = ata_id_has_hipm(dev->id); @@ -3505,7 +3504,11 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy, if (ap && ap->slave_link) ap->slave_link->lpm_policy = policy; - /* host config updated, enable DIPM if transitioning to MIN_POWER */ + /* + * Host config updated, enable DIPM if transitioning to + * ATA_LPM_MIN_POWER, ATA_LPM_MIN_POWER_WITH_PARTIAL, or + * ATA_LPM_MED_POWER_WITH_DIPM. + */ ata_for_each_dev(dev, link, ENABLED) { if (policy >= ATA_LPM_MED_POWER_WITH_DIPM && !no_dipm && ata_id_has_dipm(dev->id)) { From 62eef53ab5ede2dba18ce4c5e7d031e05ab74025 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 15 May 2025 15:56:24 +0200 Subject: [PATCH 15/18] ata: libata-eh: Add ata_eh_set_lpm() WARN_ON_ONCE link->lpm_policy is initialized to ATA_LPM_UNKNOWN in ata_eh_reset(). ata_eh_set_lpm() is then only called if link->lpm_policy != ap->target_lpm_policy (after reset) and then only if link->lpm_policy > ATA_LPM_MAX_POWER (before revalidation). This means that ata_eh_set_lpm() is currently never called with policy == ATA_LPM_UNKNOWN. Add a WARN_ON_ONCE so that it is more obvious from reading the code that this function is never called with policy == ATA_LPM_UNKNOWN. Signed-off-by: Niklas Cassel Signed-off-by: Damien Le Moal --- drivers/ata/libata-eh.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index f39756a26751..89b7b2139a16 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -3442,6 +3442,13 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy, (link->flags & ATA_LFLAG_NO_LPM) || (ap && !ap->ops->set_lpm)) return 0; + /* + * This function currently assumes that it will never be supplied policy + * ATA_LPM_UNKNOWN. + */ + if (WARN_ON_ONCE(policy == ATA_LPM_UNKNOWN)) + return 0; + /* * DIPM is enabled only for ATA_LPM_MIN_POWER, * ATA_LPM_MIN_POWER_WITH_PARTIAL, and ATA_LPM_MED_POWER_WITH_DIPM, as From 22cfba10dbfb3b4ded7038a543b74110d6d2de85 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 15 May 2025 15:56:25 +0200 Subject: [PATCH 16/18] ata: libata-eh: Rename hipm and dipm variables Rename the hipm and dipm variables to have a clearer name. Also fold in the usage of no_dipm, as that is required in order to give the dipm variable a more descriptive name. No functional change. Signed-off-by: Niklas Cassel Signed-off-by: Damien Le Moal --- drivers/ata/libata-eh.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 89b7b2139a16..dcb449edd315 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -3455,22 +3455,23 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy, * some devices misbehave when the host NACKs transition to SLUMBER. */ ata_for_each_dev(dev, link, ENABLED) { - bool hipm = ata_id_has_hipm(dev->id); - bool dipm = ata_id_has_dipm(dev->id) && !no_dipm; + bool dev_has_hipm = ata_id_has_hipm(dev->id); + bool dev_has_dipm = ata_id_has_dipm(dev->id); /* find the first enabled and LPM enabled devices */ if (!link_dev) link_dev = dev; - if (!lpm_dev && (hipm || dipm)) + if (!lpm_dev && (dev_has_hipm || (dev_has_dipm && !no_dipm))) lpm_dev = dev; hints &= ~ATA_LPM_EMPTY; - if (!hipm) + if (!dev_has_hipm) hints &= ~ATA_LPM_HIPM; /* disable DIPM before changing link config */ - if (policy < ATA_LPM_MED_POWER_WITH_DIPM && dipm) { + if (policy < ATA_LPM_MED_POWER_WITH_DIPM && + (dev_has_dipm && !no_dipm)) { err_mask = ata_dev_set_feature(dev, SETFEATURES_SATA_DISABLE, SATA_DIPM); if (err_mask && err_mask != AC_ERR_DEV) { @@ -3517,8 +3518,10 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy, * ATA_LPM_MED_POWER_WITH_DIPM. */ ata_for_each_dev(dev, link, ENABLED) { + bool dev_has_dipm = ata_id_has_dipm(dev->id); + if (policy >= ATA_LPM_MED_POWER_WITH_DIPM && !no_dipm && - ata_id_has_dipm(dev->id)) { + dev_has_dipm) { err_mask = ata_dev_set_feature(dev, SETFEATURES_SATA_ENABLE, SATA_DIPM); if (err_mask && err_mask != AC_ERR_DEV) { From 6d915e2812b3faae71d54b914f6351a562204b79 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 15 May 2025 15:56:26 +0200 Subject: [PATCH 17/18] ata: libata-eh: Rename no_dipm variable to be more clear Rename the no_dipm variable to host_has_dipm, by inverting the expression, and and also having a clearer name. No functional change. Signed-off-by: Niklas Cassel Signed-off-by: Damien Le Moal --- drivers/ata/libata-eh.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index dcb449edd315..0e36a7806cff 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -3432,7 +3432,7 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy, struct ata_eh_context *ehc = &link->eh_context; struct ata_device *dev, *link_dev = NULL, *lpm_dev = NULL; enum ata_lpm_policy old_policy = link->lpm_policy; - bool no_dipm = link->ap->flags & ATA_FLAG_NO_DIPM; + bool host_has_dipm = !(link->ap->flags & ATA_FLAG_NO_DIPM); unsigned int hints = ATA_LPM_EMPTY | ATA_LPM_HIPM; unsigned int err_mask; int rc; @@ -3462,7 +3462,8 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy, if (!link_dev) link_dev = dev; - if (!lpm_dev && (dev_has_hipm || (dev_has_dipm && !no_dipm))) + if (!lpm_dev && + (dev_has_hipm || (dev_has_dipm && host_has_dipm))) lpm_dev = dev; hints &= ~ATA_LPM_EMPTY; @@ -3471,7 +3472,7 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy, /* disable DIPM before changing link config */ if (policy < ATA_LPM_MED_POWER_WITH_DIPM && - (dev_has_dipm && !no_dipm)) { + (dev_has_dipm && host_has_dipm)) { err_mask = ata_dev_set_feature(dev, SETFEATURES_SATA_DISABLE, SATA_DIPM); if (err_mask && err_mask != AC_ERR_DEV) { @@ -3520,7 +3521,7 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy, ata_for_each_dev(dev, link, ENABLED) { bool dev_has_dipm = ata_id_has_dipm(dev->id); - if (policy >= ATA_LPM_MED_POWER_WITH_DIPM && !no_dipm && + if (policy >= ATA_LPM_MED_POWER_WITH_DIPM && host_has_dipm && dev_has_dipm) { err_mask = ata_dev_set_feature(dev, SETFEATURES_SATA_ENABLE, SATA_DIPM); From a374cfbf609017f77a985357656be07a2da22c5f Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 15 May 2025 15:56:27 +0200 Subject: [PATCH 18/18] ata: libata-eh: Keep DIPM disabled while modifying the allowed LPM states Currently, it is possible that LPM is enabled while calling the set_lpm() callback. The current code performs a SET FEATURES command to disable DIPM if policy < ATA_LPM_MED_POWER_WITH_DIPM, this means that it will currently disable DIPM for policies: ATA_LPM_UNKNOWN, ATA_LPM_MAX_POWER, ATA_LPM_MED_POWER (but not for policy ATA_LPM_MED_POWER_WITH_DIPM). The code called after calling the set_lpm() callback will later perform a SET FEATURES command to enable DIPM, if policy >= ATA_LPM_MED_POWER_WITH_DIPM. As we can see DIPM will not be disabled before calling set_lpm() if the LPM policy is: ATA_LPM_MED_POWER_WITH_DIPM, ATA_LPM_MIN_POWER_WITH_PARTIAL, or ATA_LPM_MIN_POWER. Make sure that we always disable DIPM before calling the set_lpm() callback. This is because the set_lpm() callback is the function (for AHCI) that sets the proper bits in PxSCTL.IPM, reflecting the support of the HBA. PxSCTL.IPM controls the LPM states that the device is allowed to enter. If the device tries to enter a state disabled by PxSCTL.IPM, the host will NAK the transition. If we do not disable DIPM before modifying PxSCTL.IPM, it is possible that DIPM will try (and will be allowed to) enter a LPM state that the HBA does not support (since we have not yet written PxSCTL.IPM, the HBA wasn't able to NAK the transition). While at it, remove the guard of host support for DIPM around the disabling of DIPM. While it makes sense to take host support for DIPM into account when enabling DIPM, it makes zero sense to take host support into account when disabling DIPM. If the host does not support DIPM, that is an even bigger reason why DIPM should be disabled on the device side. Signed-off-by: Niklas Cassel Signed-off-by: Damien Le Moal --- drivers/ata/libata-eh.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 0e36a7806cff..c11d8e634bf7 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -3471,8 +3471,7 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy, hints &= ~ATA_LPM_HIPM; /* disable DIPM before changing link config */ - if (policy < ATA_LPM_MED_POWER_WITH_DIPM && - (dev_has_dipm && host_has_dipm)) { + if (dev_has_dipm) { err_mask = ata_dev_set_feature(dev, SETFEATURES_SATA_DISABLE, SATA_DIPM); if (err_mask && err_mask != AC_ERR_DEV) {