[PATCH 5/6] PCI: keystone: Add PCI legacy interrupt support for AM654

Krzysztof Wilczyński kw at linux.com
Fri Mar 26 07:14:02 GMT 2021


Hi Kishon,

[...]
> +	if (!legacy_irq_domain) {
> +		dev_err(dev, "Failed to add irq domain for legacy irqs\n");
> +		return -EINVAL;
> +	}
[...]

It would be "IRQ" and "IRQs" in the message above.

[...]
> -	ret = ks_pcie_config_legacy_irq(ks_pcie);
> -	if (ret)
> -		return ret;
> +	if (!ks_pcie->is_am6) {
> +		pp->bridge->child_ops = &ks_child_pcie_ops;
> +		ret = ks_pcie_config_legacy_irq(ks_pcie);
> +		if (ret)
> +			return ret;
> +	} else {
> +		ret = ks_pcie_am654_config_legacy_irq(ks_pcie);
> +		if (ret)
> +			return ret;
> +	}
[...]

What if we change this to the following:

	if (!ks_pcie->is_am6) {
		pp->bridge->child_ops = &ks_child_pcie_ops;
		ret = ks_pcie_config_legacy_irq(ks_pcie);
	} else {
		ret = ks_pcie_am654_config_legacy_irq(ks_pcie);
	}
	
	if (ret)
	  	return ret;

Not sure if this is something you would prefer, but it seems that either
of the functions can set "ret", so checking immediately after would be
the same as checking in either of the branches.  But, this is a matter
of style, so it would be up to you - not sure what do you prefer.

Krzysztof



More information about the linux-arm-kernel mailing list