> -----Original Message----- > From: Mark Brown <broo...@kernel.org> > Sent: Wednesday, September 9, 2020 5:32 PM > To: kuldip dwivedi <kuldip.dwiv...@puresoftware.com> > Cc: Ashish Kumar <ashish.ku...@nxp.com>; Yogesh Gaur > <yogeshgaur...@gmail.com>; linux-...@vger.kernel.org; linux- > ker...@vger.kernel.org; Varun Sethi <v.se...@nxp.com>; Arokia Samy > <arokia.s...@nxp.com> > Subject: Re: [PATCH v1] spi: spi-nxp-fspi: Add ACPI support > > On Tue, Sep 08, 2020 at 11:32:27AM +0530, kuldip dwivedi wrote: > > This appears to be v2 not v1? This is separate Patch so v1 should be OK here. Earlier one was related to DSPI. https://lore.kernel.org/linux-spi/20200827113216.ga4...@sirena.org.uk/T/#t
> > > Currently NXP fspi driver has support of DT only. Adding ACPI support > > to the driver so that it can be used by UEFI firmware booting in ACPI > > mode. This driver will be probed if any firmware will expose HID > > "NXP0009" in DSDT table. > > As I said on your previous version: > > | Does NXP know about this ID assignment from their namespace? ACPI IDs > | should be namespaced by whoever's assigning the ID to avoid collisions. Yes, NXP is aware. > > Please don't ignore review comments, people are generally making them for a > reason and are likely to have the same concerns if issues remain unaddressed. > Having to repeat the same comments can get repetitive and make people question > the value of time spent reviewing. If you disagree with the review comments > that's fine but you need to reply and discuss your concerns so that the reviewer > can understand your decisions. This is new Patch for different IP (FSPI) and scenario is different from DSPI driver. > > > @@ -900,6 +910,8 @@ static int nxp_fspi_default_setup(struct nxp_fspi *f) > > return ret; > > > > /* Reset the module */ > > + fspi_writel(f, FSPI_MCR0_SWRST, (base + FSPI_MCR0)); > > + > > /* w1c register, wait unit clear */ > > ret = fspi_readl_poll_tout(f, f->iobase + FSPI_MCR0, > > FSPI_MCR0_SWRST, 0, POLL_TOUT, false); > > Why are you adding this reset? How is it connected to adding ACPI support - it > looks like it should be a separate patch. I observed a kernel panic in setting up the driver, and this fixed the issue.
