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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux