Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 26 Mar 2021 08:14:02 +0100
From:   Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>
To:     Kishon Vijay Abraham I <kishon@ti.com>
Cc:     Bjorn Helgaas <bhelgaas@google.com>,
        Rob Herring <robh+dt@kernel.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Marc Zyngier <maz@kernel.org>, linux-pci@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        Lokesh Vutla <lokeshvutla@ti.com>
Subject: Re: [PATCH 5/6] PCI: keystone: Add PCI legacy interrupt support for
 AM654
Message-ID: <YF2Jugi4SE+yKMQa@rocinante>
References: <20210325090026.8843-1-kishon@ti.com>
 <20210325090026.8843-6-kishon@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20210325090026.8843-6-kishon@ti.com>
Precedence: bulk

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