From 54091eee08acebfb5e971611c3f189e7577a1058 Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Fri, 25 Jul 2025 10:58:14 +0900 Subject: [PATCH 1/5] scsi: libsas: Refactor dev_is_sata() Use a switch statement in dev_is_sata() to make the code more readable (and probably slightly better than a series of or conditions). Also have this inline function return a boolean instead of an integer. No functional changes. Signed-off-by: Damien Le Moal Link: https://lore.kernel.org/r/20250725015818.171252-2-dlemoal@kernel.org Reviewed-by: John Garry Reviewed-by: Jason Yan Signed-off-by: Martin K. Petersen --- include/scsi/sas_ata.h | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h index 92e27e7bf088..8dddd0036f99 100644 --- a/include/scsi/sas_ata.h +++ b/include/scsi/sas_ata.h @@ -15,10 +15,17 @@ #ifdef CONFIG_SCSI_SAS_ATA -static inline int dev_is_sata(struct domain_device *dev) +static inline bool dev_is_sata(struct domain_device *dev) { - return dev->dev_type == SAS_SATA_DEV || dev->dev_type == SAS_SATA_PM || - dev->dev_type == SAS_SATA_PM_PORT || dev->dev_type == SAS_SATA_PENDING; + switch (dev->dev_type) { + case SAS_SATA_DEV: + case SAS_SATA_PENDING: + case SAS_SATA_PM: + case SAS_SATA_PM_PORT: + return true; + default: + return false; + } } int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy); @@ -49,9 +56,9 @@ static inline void sas_ata_disabled_notice(void) pr_notice_once("ATA device seen but CONFIG_SCSI_SAS_ATA=N\n"); } -static inline int dev_is_sata(struct domain_device *dev) +static inline bool dev_is_sata(struct domain_device *dev) { - return 0; + return false; } static inline int sas_ata_init(struct domain_device *dev) { From 0dd03570512a305bc44ac9c8326da95dd8fc3a1d Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Fri, 25 Jul 2025 10:58:15 +0900 Subject: [PATCH 2/5] scsi: libsas: Simplify sas_ata_wait_eh() Simplify the code of sas_ata_wait_eh(), removing the local variable ap for the pointer to the device ata_port structure. The test using dev_is_sata() is also removed as all call sites of this function check if the device is a SATA one before calling this function. Signed-off-by: Damien Le Moal Link: https://lore.kernel.org/r/20250725015818.171252-3-dlemoal@kernel.org Reviewed-by: John Garry Reviewed-by: Jason Yan Signed-off-by: Martin K. Petersen --- drivers/scsi/libsas/sas_ata.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c index 7b4e7a61965a..2cbf38b18c5c 100644 --- a/drivers/scsi/libsas/sas_ata.c +++ b/drivers/scsi/libsas/sas_ata.c @@ -927,13 +927,7 @@ EXPORT_SYMBOL_GPL(sas_ata_schedule_reset); void sas_ata_wait_eh(struct domain_device *dev) { - struct ata_port *ap; - - if (!dev_is_sata(dev)) - return; - - ap = dev->sata_dev.ap; - ata_port_wait_eh(ap); + ata_port_wait_eh(dev->sata_dev.ap); } void sas_ata_device_link_abort(struct domain_device *device, bool force_reset) From bd31394aabf36ee18781c6371e02d789484ffda3 Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Fri, 25 Jul 2025 10:58:16 +0900 Subject: [PATCH 3/5] scsi: libsas: Make sas_get_ata_info() static The function sas_get_ata_info() is used only in drivers/scsi/libsas/sas_ata.c. Remove its definition from include/scsi/sas_ata.h and make this function static. No functional changes. Signed-off-by: Damien Le Moal Link: https://lore.kernel.org/r/20250725015818.171252-4-dlemoal@kernel.org Reviewed-by: Johannes Thumshirn Reviewed-by: John Garry Reviewed-by: Jason Yan Signed-off-by: Martin K. Petersen --- drivers/scsi/libsas/sas_ata.c | 2 +- include/scsi/sas_ata.h | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c index 2cbf38b18c5c..cc093cdc9c69 100644 --- a/drivers/scsi/libsas/sas_ata.c +++ b/drivers/scsi/libsas/sas_ata.c @@ -252,7 +252,7 @@ static int sas_get_ata_command_set(struct domain_device *dev) return ata_dev_classify(&tf); } -int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy) +static int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy) { if (phy->attached_tproto & SAS_PROTOCOL_STP) dev->tproto = phy->attached_tproto; diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h index 8dddd0036f99..5e3475975aee 100644 --- a/include/scsi/sas_ata.h +++ b/include/scsi/sas_ata.h @@ -28,7 +28,6 @@ static inline bool dev_is_sata(struct domain_device *dev) } } -int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy); int sas_ata_init(struct domain_device *dev); void sas_ata_task_abort(struct sas_task *task); void sas_ata_strategy_handler(struct Scsi_Host *shost); @@ -96,11 +95,6 @@ static inline void sas_resume_sata(struct asd_sas_port *port) { } -static inline int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy) -{ - return 0; -} - static inline void sas_ata_end_eh(struct ata_port *ap) { } From 704ed03abf6b1c2752a8b16446a5ebf18694fefe Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Fri, 25 Jul 2025 10:58:17 +0900 Subject: [PATCH 4/5] scsi: libsas: Move declarations of internal functions to sas_internal.h Move the declaration of all functions used only within libsas from include/scsi/sas_ata.h to drivers/scsi/libsas/sas_internal.h. No functional changes. Signed-off-by: Damien Le Moal Link: https://lore.kernel.org/r/20250725015818.171252-5-dlemoal@kernel.org Reviewed-by: Johannes Thumshirn Reviewed-by: John Garry Reviewed-by: Jason Yan Signed-off-by: Martin K. Petersen --- drivers/scsi/libsas/sas_internal.h | 74 ++++++++++++++++++++++++++++++ include/scsi/sas_ata.h | 68 +-------------------------- 2 files changed, 75 insertions(+), 67 deletions(-) diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h index 03d6ec1eb970..16f8d81d7531 100644 --- a/drivers/scsi/libsas/sas_internal.h +++ b/drivers/scsi/libsas/sas_internal.h @@ -222,4 +222,78 @@ static inline void sas_put_device(struct domain_device *dev) kref_put(&dev->kref, sas_free_device); } +#ifdef CONFIG_SCSI_SAS_ATA + +int sas_ata_init(struct domain_device *dev); +void sas_ata_task_abort(struct sas_task *task); +int sas_discover_sata(struct domain_device *dev); +int sas_ata_add_dev(struct domain_device *parent, struct ex_phy *phy, + struct domain_device *child, int phy_id); +void sas_ata_strategy_handler(struct Scsi_Host *shost); +void sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q); +void sas_ata_end_eh(struct ata_port *ap); +void sas_ata_wait_eh(struct domain_device *dev); +void sas_probe_sata(struct asd_sas_port *port); +void sas_suspend_sata(struct asd_sas_port *port); +void sas_resume_sata(struct asd_sas_port *port); + +#else + +static inline int sas_ata_init(struct domain_device *dev) +{ + return 0; +} + +static inline void sas_ata_task_abort(struct sas_task *task) +{ +} + +static inline void sas_ata_strategy_handler(struct Scsi_Host *shost) +{ +} + +static inline void sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q) +{ +} + +static inline void sas_ata_end_eh(struct ata_port *ap) +{ +} + +static inline void sas_ata_wait_eh(struct domain_device *dev) +{ +} + +static inline void sas_probe_sata(struct asd_sas_port *port) +{ +} + +static inline void sas_suspend_sata(struct asd_sas_port *port) +{ +} + +static inline void sas_resume_sata(struct asd_sas_port *port) +{ +} + +static inline void sas_ata_disabled_notice(void) +{ + pr_notice_once("ATA device seen but CONFIG_SCSI_SAS_ATA=N\n"); +} + +static inline int sas_discover_sata(struct domain_device *dev) +{ + sas_ata_disabled_notice(); + return -ENXIO; +} + +static inline int sas_ata_add_dev(struct domain_device *parent, struct ex_phy *phy, + struct domain_device *child, int phy_id) +{ + sas_ata_disabled_notice(); + return -ENODEV; +} + +#endif + #endif /* _SAS_INTERNAL_H_ */ diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h index 5e3475975aee..a161c0222931 100644 --- a/include/scsi/sas_ata.h +++ b/include/scsi/sas_ata.h @@ -28,77 +28,24 @@ static inline bool dev_is_sata(struct domain_device *dev) } } -int sas_ata_init(struct domain_device *dev); -void sas_ata_task_abort(struct sas_task *task); -void sas_ata_strategy_handler(struct Scsi_Host *shost); -void sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q); void sas_ata_schedule_reset(struct domain_device *dev); -void sas_ata_wait_eh(struct domain_device *dev); -void sas_probe_sata(struct asd_sas_port *port); -void sas_suspend_sata(struct asd_sas_port *port); -void sas_resume_sata(struct asd_sas_port *port); -void sas_ata_end_eh(struct ata_port *ap); void sas_ata_device_link_abort(struct domain_device *dev, bool force_reset); -int sas_execute_ata_cmd(struct domain_device *device, u8 *fis, - int force_phy_id); +int sas_execute_ata_cmd(struct domain_device *device, u8 *fis, int force_phy_id); int smp_ata_check_ready_type(struct ata_link *link); -int sas_discover_sata(struct domain_device *dev); -int sas_ata_add_dev(struct domain_device *parent, struct ex_phy *phy, - struct domain_device *child, int phy_id); extern const struct attribute_group sas_ata_sdev_attr_group; #else -static inline void sas_ata_disabled_notice(void) -{ - pr_notice_once("ATA device seen but CONFIG_SCSI_SAS_ATA=N\n"); -} - static inline bool dev_is_sata(struct domain_device *dev) { return false; } -static inline int sas_ata_init(struct domain_device *dev) -{ - return 0; -} -static inline void sas_ata_task_abort(struct sas_task *task) -{ -} - -static inline void sas_ata_strategy_handler(struct Scsi_Host *shost) -{ -} - -static inline void sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q) -{ -} static inline void sas_ata_schedule_reset(struct domain_device *dev) { } -static inline void sas_ata_wait_eh(struct domain_device *dev) -{ -} - -static inline void sas_probe_sata(struct asd_sas_port *port) -{ -} - -static inline void sas_suspend_sata(struct asd_sas_port *port) -{ -} - -static inline void sas_resume_sata(struct asd_sas_port *port) -{ -} - -static inline void sas_ata_end_eh(struct ata_port *ap) -{ -} - static inline void sas_ata_device_link_abort(struct domain_device *dev, bool force_reset) { @@ -115,19 +62,6 @@ static inline int smp_ata_check_ready_type(struct ata_link *link) return 0; } -static inline int sas_discover_sata(struct domain_device *dev) -{ - sas_ata_disabled_notice(); - return -ENXIO; -} - -static inline int sas_ata_add_dev(struct domain_device *parent, struct ex_phy *phy, - struct domain_device *child, int phy_id) -{ - sas_ata_disabled_notice(); - return -ENODEV; -} - #define sas_ata_sdev_attr_group ((struct attribute_group) {}) #endif From 75fe230b9bed364d7ddca482ff29979d873718fa Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Fri, 25 Jul 2025 10:58:18 +0900 Subject: [PATCH 5/5] scsi: libsas: Use a bool for sas_deform_port() second argument Change the type of the "gone" argument of sas_deform_port() from int to bool. Simliarly, to be consistent, do the same change to the function sas_unregister_domain_devices(). No functional changes. Signed-off-by: Damien Le Moal Link: https://lore.kernel.org/r/20250725015818.171252-6-dlemoal@kernel.org Reviewed-by: Johannes Thumshirn Reviewed-by: John Garry Reviewed-by: Jason Yan Signed-off-by: Martin K. Petersen --- drivers/scsi/libsas/sas_discover.c | 2 +- drivers/scsi/libsas/sas_internal.h | 4 ++-- drivers/scsi/libsas/sas_phy.c | 6 +++--- drivers/scsi/libsas/sas_port.c | 13 ++++++------- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c index 951bdc554a10..b07062db50b2 100644 --- a/drivers/scsi/libsas/sas_discover.c +++ b/drivers/scsi/libsas/sas_discover.c @@ -406,7 +406,7 @@ void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev) } } -void sas_unregister_domain_devices(struct asd_sas_port *port, int gone) +void sas_unregister_domain_devices(struct asd_sas_port *port, bool gone) { struct domain_device *dev, *n; diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h index 16f8d81d7531..6706f2be8d27 100644 --- a/drivers/scsi/libsas/sas_internal.h +++ b/drivers/scsi/libsas/sas_internal.h @@ -44,7 +44,7 @@ void sas_hash_addr(u8 *hashed, const u8 *sas_addr); int sas_discover_root_expander(struct domain_device *dev); int sas_ex_revalidate_domain(struct domain_device *dev); -void sas_unregister_domain_devices(struct asd_sas_port *port, int gone); +void sas_unregister_domain_devices(struct asd_sas_port *port, bool gone); void sas_init_disc(struct sas_discovery *disc, struct asd_sas_port *port); void sas_discover_event(struct asd_sas_port *port, enum discover_event ev); @@ -70,7 +70,7 @@ void sas_enable_revalidation(struct sas_ha_struct *ha); void sas_queue_deferred_work(struct sas_ha_struct *ha); void __sas_drain_work(struct sas_ha_struct *ha); -void sas_deform_port(struct asd_sas_phy *phy, int gone); +void sas_deform_port(struct asd_sas_phy *phy, bool gone); void sas_porte_bytes_dmaed(struct work_struct *work); void sas_porte_broadcast_rcvd(struct work_struct *work); diff --git a/drivers/scsi/libsas/sas_phy.c b/drivers/scsi/libsas/sas_phy.c index 57494ac97076..635835c28ecd 100644 --- a/drivers/scsi/libsas/sas_phy.c +++ b/drivers/scsi/libsas/sas_phy.c @@ -20,7 +20,7 @@ static void sas_phye_loss_of_signal(struct work_struct *work) struct asd_sas_phy *phy = ev->phy; phy->error = 0; - sas_deform_port(phy, 1); + sas_deform_port(phy, true); } static void sas_phye_oob_done(struct work_struct *work) @@ -40,7 +40,7 @@ static void sas_phye_oob_error(struct work_struct *work) struct sas_internal *i = to_sas_internal(sas_ha->shost->transportt); - sas_deform_port(phy, 1); + sas_deform_port(phy, true); if (!port && phy->enabled && i->dft->lldd_control_phy) { phy->error++; @@ -85,7 +85,7 @@ static void sas_phye_resume_timeout(struct work_struct *work) phy->error = 0; phy->suspended = 0; - sas_deform_port(phy, 1); + sas_deform_port(phy, true); } diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c index e3f2ed913419..de7556070048 100644 --- a/drivers/scsi/libsas/sas_port.c +++ b/drivers/scsi/libsas/sas_port.c @@ -113,7 +113,7 @@ static void sas_form_port(struct asd_sas_phy *phy) if (port) { if (!phy_is_wideport_member(port, phy)) - sas_deform_port(phy, 0); + sas_deform_port(phy, false); else if (phy->suspended) { phy->suspended = 0; sas_resume_port(phy); @@ -206,7 +206,7 @@ static void sas_form_port(struct asd_sas_phy *phy) * This is called when the physical link to the other phy has been * lost (on this phy), in Event thread context. We cannot delay here. */ -void sas_deform_port(struct asd_sas_phy *phy, int gone) +void sas_deform_port(struct asd_sas_phy *phy, bool gone) { struct sas_ha_struct *sas_ha = phy->ha; struct asd_sas_port *port = phy->port; @@ -301,7 +301,7 @@ void sas_porte_link_reset_err(struct work_struct *work) struct asd_sas_event *ev = to_asd_sas_event(work); struct asd_sas_phy *phy = ev->phy; - sas_deform_port(phy, 1); + sas_deform_port(phy, true); } void sas_porte_timer_event(struct work_struct *work) @@ -309,7 +309,7 @@ void sas_porte_timer_event(struct work_struct *work) struct asd_sas_event *ev = to_asd_sas_event(work); struct asd_sas_phy *phy = ev->phy; - sas_deform_port(phy, 1); + sas_deform_port(phy, true); } void sas_porte_hard_reset(struct work_struct *work) @@ -317,7 +317,7 @@ void sas_porte_hard_reset(struct work_struct *work) struct asd_sas_event *ev = to_asd_sas_event(work); struct asd_sas_phy *phy = ev->phy; - sas_deform_port(phy, 1); + sas_deform_port(phy, true); } /* ---------- SAS port registration ---------- */ @@ -358,8 +358,7 @@ void sas_unregister_ports(struct sas_ha_struct *sas_ha) for (i = 0; i < sas_ha->num_phys; i++) if (sas_ha->sas_phy[i]->port) - sas_deform_port(sas_ha->sas_phy[i], 0); - + sas_deform_port(sas_ha->sas_phy[i], false); } const work_func_t sas_port_event_fns[PORT_NUM_EVENTS] = {