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

Manivannan Sadhasivam
2025-02-24T13:18:37Z

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

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