On 4/1/23 17:15, Jason Yan wrote:
> In sas_check_eeds() there is an empty branch. We can reverse the
> test expression and then remove the empty branch. Also the the test
> expression is a little bit complex so it deserves an individual
> function. And make the continuing prototype lines indented after
> the opening parenthesis to follow the standard coding style.
>
> Signed-off-by: Jason Yan <yanaijie@xxxxxxxxxx>
> ---
> drivers/scsi/libsas/sas_expander.c | 38 ++++++++++++++----------------
> 1 file changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index dc670304f181..048a931d856a 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -1198,37 +1198,35 @@ static void sas_print_parent_topology_bug(struct domain_device *child,
> sas_route_char(child, child_phy));
> }
>
> +static bool sas_eeds_valid(struct domain_device *parent, struct domain_device *child)
> +{
> + struct sas_discovery *disc = &parent->port->disc;
Missing blank line after declaration.
> + return (((SAS_ADDR(disc->eeds_a) == SAS_ADDR(parent->sas_addr)) ||
> + (SAS_ADDR(disc->eeds_a) == SAS_ADDR(child->sas_addr))) &&
> + ((SAS_ADDR(disc->eeds_b) == SAS_ADDR(parent->sas_addr)) ||
> + (SAS_ADDR(disc->eeds_b) == SAS_ADDR(child->sas_addr))));
Drop the inner-most and outter-most parenthesis.
> +}
> +
> static int sas_check_eeds(struct domain_device *child,
> - struct ex_phy *parent_phy,
> - struct ex_phy *child_phy)
> + struct ex_phy *parent_phy,
> + struct ex_phy *child_phy)
> {
> int res = 0;
> struct domain_device *parent = child->parent;
> + struct sas_discovery *disc = &parent->port->disc;
>
> - if (SAS_ADDR(parent->port->disc.fanout_sas_addr) != 0) {
> + if (SAS_ADDR(disc->fanout_sas_addr) != 0) {
> res = -ENODEV;
> pr_warn("edge ex %016llx phy S:%02d <--> edge ex %016llx phy S:%02d, while there is a fanout ex %016llx\n",
> SAS_ADDR(parent->sas_addr),
> parent_phy->phy_id,
> SAS_ADDR(child->sas_addr),
> child_phy->phy_id,
> - SAS_ADDR(parent->port->disc.fanout_sas_addr));
> - } else if (SAS_ADDR(parent->port->disc.eeds_a) == 0) {
> - memcpy(parent->port->disc.eeds_a, parent->sas_addr,
> - SAS_ADDR_SIZE);
> - memcpy(parent->port->disc.eeds_b, child->sas_addr,
> - SAS_ADDR_SIZE);
> - } else if (((SAS_ADDR(parent->port->disc.eeds_a) ==
> - SAS_ADDR(parent->sas_addr)) ||
> - (SAS_ADDR(parent->port->disc.eeds_a) ==
> - SAS_ADDR(child->sas_addr)))
> - &&
> - ((SAS_ADDR(parent->port->disc.eeds_b) ==
> - SAS_ADDR(parent->sas_addr)) ||
> - (SAS_ADDR(parent->port->disc.eeds_b) ==
> - SAS_ADDR(child->sas_addr))))
> - ;
> - else {
> + SAS_ADDR(disc->fanout_sas_addr));
> + } else if (SAS_ADDR(disc->eeds_a) == 0) {
> + memcpy(disc->eeds_a, parent->sas_addr, SAS_ADDR_SIZE);
> + memcpy(disc->eeds_b, child->sas_addr, SAS_ADDR_SIZE);
> + } else if (!sas_eeds_valid(parent, child)) {
> res = -ENODEV;
> pr_warn("edge ex %016llx phy%02d <--> edge ex %016llx phy%02d link forms a third EEDS!\n",
> SAS_ADDR(parent->sas_addr),
--
Damien Le Moal
Western Digital Research
