Hi, Thank you for your review. On Thu, Aug 05, 2021 at 11:59:08AM +0100, Lorenzo Pieralisi wrote: > On Sat, Jul 24, 2021 at 07:14:20AM +0900, Nobuhiro Iwamatsu wrote: > > Add support to PCIe RC controller on Toshiba Visconti ARM SoCs. PCIe > > controller is based of Synopsys DesignWare PCIe core. > > > > This patch does not yet use the clock framework to control the clock. > > This will be replaced in the future. > > This is not relevant information. I expect the commit log to describe > the change and the reasons behind the choices. OK, I will drop this sentence. > > Speaking of which, I'd like to understand what > > > This patch does not yet use the clock framework to control the clock. > > means and why it can't be done within this series. Visconti5 has a clock control IP, but the driver for this is still under development and has not been applied into mainline. Instead, the clock for this driver is running using DT's fixed-clock. This will be replaced by a clock driver. > > > Signed-off-by: Yuji Ishikawa <yuji2.ishikawa@xxxxxxxxxxxxx> > > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@xxxxxxxxxxxxx> > > > > --- > > drivers/pci/controller/dwc/Kconfig | 9 + > > drivers/pci/controller/dwc/Makefile | 1 + > > drivers/pci/controller/dwc/pcie-visconti.c | 333 +++++++++++++++++++++ > > 3 files changed, 343 insertions(+) > > create mode 100644 drivers/pci/controller/dwc/pcie-visconti.c > > <snip> > > + > > +static int > > +visconti_add_pcie_port(struct visconti_pcie *pcie, struct platform_device *pdev) > > Nit: don't split lines like this, it is better to keep return value > type and function name in one single line. > > Do it like this: > > static int visconti_add_pcie_port(struct visconti_pcie *pcie, > struct platform_device *pdev) > OK, I will fix in v6. > Lorenzo > Best regards, Nobuhiro