On Tue, Jun 14, 2022 at 11:58:46AM +0000, Yoshihiro Shimoda wrote:
> Hi Rob,
>
> Thank you for your review!
>
> > From: Rob Herring, Sent: Tuesday, June 14, 2022 6:51 AM
> >
> > On Mon, Jun 13, 2022 at 08:57:09PM +0900, Yoshihiro Shimoda wrote:
> > > Add R-Car Gen4 PCIe Endpoint support. This controller is based on
> > > Synopsys Designware PCIe.
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> > > ---
[...]
> > > + u32 num_lanes;
> >
> > What's wrong with dw_pcie.num_lanes?
>
> The dw_pcie.num_lanes is set after dw_pcie_ep_init() succeeded.
> However, this driver would like to refer the num_lanes before dw_pcie_ep_init()
> to initialize vendor-specific registers. In other words, this value is only
> needed at that timing. So, we can remove this from struct rcar_gen4_pcie_ep,
> and just get the num_lanes as a local variable.
AFAICT, you are just using it to set PCI_EXP_LNKCAP_MLW. That's a common
register, so it should be able to set in the common code.
[...]
> > > +static struct platform_driver rcar_gen4_pcie_ep_driver = {
> > > + .driver = {
> > > + .name = "pcie-rcar-gen4-ep",
> > > + .of_match_table = rcar_gen4_pcie_of_match,
> > > + },
> > > + .probe = rcar_gen4_pcie_ep_probe,
> > > + .remove = rcar_gen4_pcie_ep_remove,
> > > +};
> > > +builtin_platform_driver(rcar_gen4_pcie_ep_driver);
> >
> > Not a module or...
> >
> > > +
> > > +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe endpoint controller driver");
> > > +MODULE_LICENSE("GPL v2");
> >
> > A module? Should be a module if possible.
>
> Oops. I'll drop these MODULE_*.
No, is there some reason it can't be a module?
Rob
