Re: [PATCH v3 2/2] PCI: mediatek-gen3: Configure PBUS_CSR registers for EN7581 SoC

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Mon, Feb 24, 2025 at 11:05:24AM +0100, Lorenzo Bianconi wrote:
> > On Sat, Feb 22, 2025 at 11:43:45AM +0100, Lorenzo Bianconi wrote:
> 
> [...]
> 
> > > +
> > > +	entry = resource_list_first_type(&host->windows, IORESOURCE_MEM);
> > > +	if (!entry)
> > > +		return -EINVAL;
> > 
> > -ENODEV or -ENOMEM
> 
> ack, I will fix it in v4
> 
> > 
> > > +
> > > +	addr = entry->res->start - entry->offset;
> > > +	err = regmap_write(pbus_regmap, args[0], lower_32_bits(addr));
> > > +	if (err)
> > 
> > MMIO write is not supposed to fail.
> 
> ack, I will fix it in v4
> 
> > 
> > > +		return err;
> > > +
> > > +	size = lower_32_bits(resource_size(entry->res));
> > > +	mask = size ? GENMASK(31, __fls(size)) : 0;
> > 
> > Size of MEM region could be 0?
> 
> I added this check since we consider just lower_32_bits().
> Do you think we should remove it?
> 

Yes please.

> > 
> > > +	err = regmap_write(pbus_regmap, args[1], mask);
> > > +	if (err)
> > 
> > MMIO write is not supposed to fail.
> 
> ack, I will fix it in v4
> 
> BTW I will remove your Reviwed-by tag since the patch has changed
> with respect to the one you added it. Please let me know if you
> prepfer to keep it.
> 

Sure, fine with me.

- Mani

-- 
மணிவண்ணன் சதாசிவம்