Re: [PATCH 4/7] PCI: renesas: Add R-Car Gen4 PCIe Endpoint support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux