[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 9 Sep 2020 18:29:10 +0530
From: Kuldip Dwivedi <kuldip.dwivedi@...esoftware.com>
To: Mark Brown <broonie@...nel.org>
Cc: Ashish Kumar <ashish.kumar@....com>,
Yogesh Gaur <yogeshgaur.83@...il.com>,
linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
Varun Sethi <V.Sethi@....com>,
Arokia Samy <arokia.samy@....com>
Subject: RE: [PATCH v1] spi: spi-nxp-fspi: Add ACPI support
> -----Original Message-----
> From: Mark Brown <broonie@...nel.org>
> Sent: Wednesday, September 9, 2020 5:32 PM
> To: kuldip dwivedi <kuldip.dwivedi@...esoftware.com>
> Cc: Ashish Kumar <ashish.kumar@....com>; Yogesh Gaur
> <yogeshgaur.83@...il.com>; linux-spi@...r.kernel.org; linux-
> kernel@...r.kernel.org; Varun Sethi <V.Sethi@....com>; Arokia Samy
> <arokia.samy@....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.GA4674@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.
Powered by blists - more mailing lists