ata: ahci: Make ahci_ignore_port() handle empty mask_port_map
Niklas Cassel
maintainerPATCH 1/1 ata: ahci: Make ahci_ignore_port() handle empty mask_port_map
Commit 8c87215dd3a2 ("ata: libahci_platform: support non-consecutive port numbers") added a skip to ahci_platform_enable_phys() for ports that are not in mask_port_map.
The code in ahci_platform_get_resources(), will currently set mask_port_map for each child "port" node it finds in the device tree.
However, device trees that do not have any child "port" nodes will not have mask_port_map set, and for non-device tree platforms mask_port_map will only exist as a quirk for specific PCI device + vendor IDs, or as a kernel module parameter, but will not be set by default.
Therefore, the common thing is that mask_port_map is only set if you do not want to use all ports (as defined by Offset 0Ch: PI – Ports Implemented register), but instead only want to use the ports in mask_port_map. If mask_port_map is not set, all ports are available.
Thus, ahci_ignore_port() must be able to handle an empty mask_port_map.
--- drivers/ata/ahci.h | 8 ++++++-- drivers/ata/libahci.c | 1 + 2 files changed, 7 insertions(+), 2 deletions(-)
drivers/ata/ahci.h +6 -2
@@ -386,8 +386,12 @@ struct ahci_host_priv {static inline bool ahci_ignore_port (struct ahci_host_priv *hpriv,unsigned int portid) { -return portid >= hpriv->nports || - !(hpriv->mask_port_map & (1 << portid)); +if (portid >= hpriv->nports) +return true; +/* mask_port_map not set means that all ports are available */ +if (!hpriv->mask_port_map) +return false; +return !(hpriv->mask_port_map & (1 << portid)); }extern int ahci_ignore_sss;
drivers/ata/libahci.c +1 -0
@@ -541,6 +541,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv) hpriv->saved_port_map = port_map; } +/* mask_port_map not set means that all ports are available */ if (hpriv->mask_port_map) { dev_warn(dev, "masking port_map 0x%lx -> 0x%lx\n", port_map,
Signature · 1 line
2.48.1
Quoted 19 lines · Niklas Cassel
Commit
8c87215dd3a2("ata:libahci_platform: support non-consecutive port numbers") added a skip toahci_platform_enable_phys()for ports that are not inmask_port_map.The code in
ahci_platform_get_resources(), will currently setmask_port_mapfor each child "port" node it finds in the device tree.However, device trees that do not have any child "port" nodes will not have
mask_port_mapset, and for non-device tree platformsmask_port_mapwill only exist as a quirk for specific PCI device + vendor IDs, or as a kernel module parameter, but will not be set by default.Therefore, the common thing is that
mask_port_mapis only set if you do not want to use all ports (as defined by Offset 0Ch: PI – Ports Implemented register), but instead only want to use the ports inmask_port_map. Ifmask_port_mapis not set, all ports are available.Thus,
ahci_ignore_port()must be able to handle an emptymask_port_map.
Quoted 7 lines
Reported-by Marek Szyprowski <[email protected]>Co-developed-by Damien Le Moal <[email protected]>Signed-off-by Damien Le Moal <[email protected]>Fixes 8c87215dd3a2 ("ata: libahci_platform: support non-consecutive port numbers")Fixes 2c202e6c4f4d ("ata: libahci_platform: Do not set mask_port_map when not needed")Signed-off-by Niklas Cassel <[email protected]>
This fixes the issue I've observed.
Quoted 29 lines
--- drivers/ata/ahci.h | 8 ++++++-- drivers/ata/libahci.c | 1 + 2 files changed, 7 insertions(+), 2 deletions(-)drivers/ata/ahci.h +6
-2@@ -386,8 +386,12 @@ struct ahci_host_priv {static inline bool ahci_ignore_port (struct ahci_host_priv *hpriv,unsigned int portid) { -return portid >= hpriv->nports || - !(hpriv->mask_port_map & (1 << portid)); +if (portid >= hpriv->nports) +return true; +/* mask_port_map not set means that all ports are available */ +if (!hpriv->mask_port_map) +return false; +return !(hpriv->mask_port_map & (1 << portid)); }extern int ahci_ignore_sss;drivers/ata/libahci.c +0
-0
drivers/ata/libahci.c void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv) · line 547
Quoted 7 lines
drivers/ata/libahci.c +1
-0@@ -541,6 +541,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv) hpriv->saved_port_map = port_map; } +/* mask_port_map not set means that all ports are available */ if (hpriv->mask_port_map) { dev_warn(dev, "masking port_map 0x%lx -> 0x%lx\n", port_map,
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
Damien Le Moal
maintainerQuoted 17 lines · Niklas Cassel
Commit
8c87215dd3a2("ata:libahci_platform: support non-consecutive port numbers") added a skip toahci_platform_enable_phys()for ports that are not inmask_port_map.The code in
ahci_platform_get_resources(), will currently setmask_port_mapfor each child "port" node it finds in the device tree.However, device trees that do not have any child "port" nodes will not have
mask_port_mapset, and for non-device tree platformsmask_port_mapwill only exist as a quirk for specific PCI device + vendor IDs, or as a kernel module parameter, but will not be set by default.Therefore, the common thing is that
mask_port_mapis only set if you do not want to use all ports (as defined by Offset 0Ch: PI – Ports Implemented register), but instead only want to use the ports inmask_port_map. Ifmask_port_mapis not set, all ports are available.
Quoted 8 lines
Thus,
ahci_ignore_port()must be able to handle an emptymask_port_map.Reported-by Marek Szyprowski <[email protected]>Co-developed-by Damien Le Moal <[email protected]>Signed-off-by Damien Le Moal <[email protected]>Fixes 8c87215dd3a2 ("ata: libahci_platform: support non-consecutive port numbers")Fixes 2c202e6c4f4d ("ata: libahci_platform: Do not set mask_port_map when not needed")
I think that it also would be a good idea to add:
So that this patch also gets picked up for backporting.
Quoted 37 lines
Signed-off-by Niklas Cassel <[email protected]>--- drivers/ata/ahci.h | 8 ++++++-- drivers/ata/libahci.c | 1 + 2 files changed, 7 insertions(+), 2 deletions(-)drivers/ata/ahci.h +6
-2@@ -386,8 +386,12 @@ struct ahci_host_priv {static inline bool ahci_ignore_port (struct ahci_host_priv *hpriv,unsigned int portid) { -return portid >= hpriv->nports || - !(hpriv->mask_port_map & (1 << portid)); +if (portid >= hpriv->nports) +return true; +/* mask_port_map not set means that all ports are available */ +if (!hpriv->mask_port_map) +return false; +return !(hpriv->mask_port_map & (1 << portid)); }extern int ahci_ignore_sss;drivers/ata/libahci.c +1
-0@@ -541,6 +541,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv) hpriv->saved_port_map = port_map; } +/* mask_port_map not set means that all ports are available */ if (hpriv->mask_port_map) { dev_warn(dev, "masking port_map 0x%lx -> 0x%lx\n", port_map,
Signature · 2 lines
Damien Le Moal Western Digital Research
Quoted 8 lines · Niklas Cassel
Commit
8c87215dd3a2("ata:libahci_platform: support non-consecutive port numbers") added a skip toahci_platform_enable_phys()for ports that are not inmask_port_map.The code in
ahci_platform_get_resources(), will currently setmask_port_mapfor each child "port" node it finds in the device tree.[...]
Applied to libata/linux.git (for-6.14), thanks!
[1/1] ata: ahci: Make ahci_ignore_port() handle empty mask_port_map
https://git.kernel.org/libata/linux/c/130ff5c8
Kind regards,
Niklas