Re: [PATCH v4 5/7] soc: aspeed: add host-side PCIe BMC device driver

Grégoire Layet <[email protected]>
Newsgroups org.kernel.vger.linux-serial,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.ozlabs.lists.linux-aspeed
Message-ID <CAFi2wKaO9ea77KP3hjoaruMgU8oVbbtuAmXK8p-12HRdRFOACQ@mail.gmail.com>
Hi Jiri,

> > ...
> > +
> > +enum aspeed_platform_id {
> > +     ASPEED,
>
> What is this good for?

Unecessary, it's removed now.

>
> > +};
> > +
> > +static const int vuart_msi_index[2] = { 16, 17 };
> > +static const int vuart_port_addr[2] = {0x3f8, 0x2f8};
>
> Sort of inconsistent spaces. Both arrays should be unsigned anyway. And
> for the latter, u16 should be enough.

True. Acknowledged.

> > ...
> > +
> > +     vuart_ioport = vuart_port_addr[idx];
> > +     /* ASPEED BMC device shift addresses by 2 to the left */
> > +     vuart_ioport = vuart_ioport << 2;
>
> Simply:
> vuart_ioport <<= 2;
> ? Or join the two lines?

Joined the two lines.

> > ...
> > +static struct pci_device_id aspeed_host_bmc_dev_pci_ids[] = {
> > +     { PCI_DEVICE(PCI_VENDOR_ID_ASPEED, PCI_BMC_DEVICE_ID),
> > +             .class = 0xFF0000, .class_mask = 0xFFFF00,
>
> PCI_CLASS_OTHERS << 16

Acknowledged

> > ...
> thanks,
> --
> js
> suse labs

Thanks for the review. Applied everything for the next revision.

Regards,
Grégoire
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.