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.