Re: [PATCH v4 2/4] PCI: dwc: Improve msg_atu_index error handling

[Niklas Cassel <cassel@kernel.org>]

On Sat, Jan 24, 2026 at 05:50:06AM +1100, Damien Le Moal wrote:
> On 2026/01/24 5:28, Niklas Cassel wrote:
> > Only try to dedicate an outbound iATU to MSG TLP if use_atu_msg is set.
> > If use_atu_msg is not set, it is completely useless to bump the index.
> > 
> > Additionally, if use_atu_msg is set, and there is no outbound iATU
> > available, return an error.
> > 
> > Fixes: e1a4ec1a9520 ("PCI: dwc: Add generic MSG TLP support for sending PME_Turn_Off when system suspend")
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>
> > Signed-off-by: Niklas Cassel <cassel@kernel.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware-host.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index ae5f2d8a3857..d7f57d77bdf5 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -982,7 +982,14 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> >  		dev_warn(pci->dev, "Ranges exceed outbound iATU size (%d)\n",
> >  			 pci->num_ob_windows);
> >  
> > -	pp->msg_atu_index = ++i;
> > +	if (pp->use_atu_msg) {
> > +		if (pci->num_ob_windows > ++i) {
> 
> 	if (pp->use_atu_msg) {
> 		i++;
> 		if (pci->num_ob_windows > i) {
> 
> is far more readable in my opinion.

Yes, I agree that it is more readable.

However, in patch 3/4 in series, I'm do a cleanup of this whole function,
not just this statement.

This patch changes the code in a way that is consistent with the existing
coding style of this function, see e.g.:
https://github.com/torvalds/linux/blob/v6.19-rc7/drivers/pci/controller/dwc/pcie-designware-host.c#L932-L934

Then patch 3/4 modifies the whole function to be more readable.

So I prefer to still keep this patch as is, such the code is always in a
"consistent coding style".

And then do all cleanups in the cleanup patch.


> 
> > +			pp->msg_atu_index = i;
> > +		} else {
> > +			dev_err(pci->dev, "Cannot add outbound window for MSG TLP\n");
> > +			return -ENOMEM;
> > +		}
> > +	}
> 
> Seeing this, I do not really see the point of the previous patch since you
> completely nuke that change here. So maybe drop patch 1 ?

My thinking was that the previous patch has
CC: stable, so I wanted the smallest viable backportable fix.

But, I guess that the "if (pp->use_atu_msg) {" guard makes sense even for a
backported fix, as it is pointless to increment the counter if use_atu_msg
is not set, so sure, I can squash patch 1 and 2.


Kind regards,
Niklas