[RFC PATCH 1/2] pci: Add TI K3 Cadence PCIe Controller

Siddharth Vadapalli s-vadapalli at ti.com
Thu Aug 22 07:40:45 CEST 2024


On Wed, Aug 21, 2024 at 12:00:20PM -0600, Tom Rini wrote:
> On Wed, Aug 21, 2024 at 08:29:07PM +0530, Siddharth Vadapalli wrote:
> 

[...]

> > +
> > +#include <asm/global_data.h>
> > +#include <asm/gpio.h>
> > +#include <clk-uclass.h>
> > +#include <dm.h>
> > +#include <dm/device_compat.h>
> > +#include <generic-phy.h>
> > +#include <linux/bitops.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> > +#include <linux/log2.h>
> > +#include <log.h>
> > +#include <pci.h>
> > +#include <power-domain.h>
> > +#include <regmap.h>
> > +#include <syscon.h>
> 
> Please audit this list to make sure you need everything because..
> 
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> 
> You don't reference gd in this code at all, so don't need this.

I missed dropping this. I had checked that the rest of the includes are
required, but I will re-check when I post the v2 patch.

> 
> So please also make sure the giant list of defines is needed.

During the development of this driver, I had committed the changes with
around 15 commits that had the defines added only when needed. Therefore
I am positive that they are required. I squashed those commits into a
single one which is this patch. Nevertheless, I will double check before
posting the v2 patch.

> > +#define usleep_range(a, b) udelay((b))
> 
> Per checkpatch.pl.conf:
> # Not Linux, so we don't recommend usleep_range() over udelay()

Sure, I will switch to udelay().

Thank you for reviewing this patch and providing feedback. I will implement
your suggestions in the v2 patch.

Regards,
Siddharth.


More information about the U-Boot mailing list